Skip to content

Commit

Permalink
Fix: do not parse empty srcset (#351)
Browse files Browse the repository at this point in the history
* Fix: don't use empty srcset

* Fix: throw when no rules in srcset
  • Loading branch information
eight04 authored Aug 12, 2024
1 parent 39f4b4e commit 65db0fb
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 45 deletions.
1 change: 0 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"rules": {
"max-statements-per-line": [2, {"max": 1}],
"no-use-before-define": [2, "nofunc"],
"semi": [2, "always"],
"no-console": ["error", {"allow": ["warn", "error"]}]
},
"extends": [
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
node_modules
web-ext-artifacts
build
chrome

*.zip
*.xpi
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"lint": "eslint --ext .js,.mjs . --cache",
"test": "npm run lint && npm run build && web-ext lint",
"start": "web-ext run",
"start-chrome": "web-ext run --target chromium",
"build": "sync-version src/static/manifest.json && shx rm -rf build/* && rollup -c",
"build-locales": "tx pull -a --use-git-timestamps && webext-tx-fix -s src/static",
"build-artifact": "shx rm -rf web-ext-artifacts/* && web-ext build --ignore-files **/*.js.map",
Expand All @@ -19,7 +20,8 @@
"changelog": "shx cat README.md | mkcat | mkql \"[content=Changelog] + ul > :first-child > *\" | mkout"
},
"eslintIgnore": [
"build"
"build",
"chrome"
],
"devDependencies": {
"@eight04/idb-storage": "^0.4.2",
Expand Down
74 changes: 34 additions & 40 deletions src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,59 +255,53 @@ function createDynamicIcon({file, enabled, onupdate}) {
return {update};
}

function pickImages(tabId, frameId = 0) {
async function pickImages(tabId, frameId = 0) {
const result = {
tabId,
frames: [],
env: null
};
return Promise.all([
await Promise.all([
getImages(),
getEnv(),
pref.get("collectFromFrames") && getImagesFromChildFrames()
]).then(() => {
// make sure the main frame is the first frame
const index = result.frames.findIndex(f => f.frameId === frameId);
if (index !== 0) {
const mainFrame = result.frames[index];
result.frames[index] = result.frames[0];
result.frames[0] = mainFrame;
}
return result;
});
]);
// make sure the main frame is the first frame
const index = result.frames.findIndex(f => f.frameId === frameId);
if (index !== 0) {
const mainFrame = result.frames[index];
result.frames[index] = result.frames[0];
result.frames[0] = mainFrame;
}
return result;

function getImages() {
return browser.tabs.sendMessage(tabId, {method: "getImages"}, {frameId})
.then(images => {
result.frames.push({
frameId,
images
});
});
async function getImages() {
const images = await browser.tabs.sendMessage(tabId, {method: "getImages"}, {frameId})
result.frames.push({
frameId,
images
});
}

function getEnv() {
return browser.tabs.sendMessage(tabId, {method: "getEnv"}, {frameId})
.then(env => {
result.env = env;
});
async function getEnv() {
const env = await browser.tabs.sendMessage(tabId, {method: "getEnv"}, {frameId})
result.env = env;
}

function getImagesFromChildFrames() {
return getChildFrames()
.then(frameIds =>
Promise.all(frameIds.map(frameId =>
browser.tabs.sendMessage(tabId, {method: "getImages"}, {frameId})
.then(images => {
result.frames.push({
frameId,
images
});
})
// https://github.com/eight04/image-picka/issues/100
.catch(console.warn)
))
);
async function getImagesFromChildFrames() {
const frameIds = await getChildFrames();
await Promise.all(frameIds.map(frameId => async () => {
try {
const images = await browser.tabs.sendMessage(tabId, {method: "getImages"}, {frameId});
result.frames.push({
frameId,
images
});
} catch (err) {
// https://github.com/eight04/image-picka/issues/100
console.warn(err);
}
}));
}

function getChildFrames() {
Expand Down
26 changes: 23 additions & 3 deletions src/lib/image-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ function update() {

function getSrcFromSrcset(set) {
const rules = parseSrcset(set);
if (!rules.length) {
throw new Error("No rules in srcset");
}
let maxRule;
for (const rule of rules) {
// FIXME: what if the rules have both density and width?
Expand All @@ -39,7 +42,11 @@ export function getSrc(img) {
// prefer srcset first
// https://www.harakis.com/hara-elite/large-2br-apartment-gallery/
if (img.srcset) {
return (getSrcFromSrcset(img.srcset));
try {
return getSrcFromSrcset(img.srcset);
} catch (err) {
console.warn("Error parsing srcset", img.srcset);
}
}
if (img.src) {
return img.src;
Expand Down Expand Up @@ -97,11 +104,24 @@ function getSrcFromPicture(el) {
source = s;
break;
}
if (source) return getSrcFromSrcset(source.srcset);
if (source && source.srcset) {
try {
return getSrcFromSrcset(source.srcset);
} catch (err) {
console.warn("Error parsing srcset", source.srcset);
}
}
const img = el.querySelector("img");
if (img) return getSrc(img);
if (allSources.length) {
return getSrcFromSrcset(allSources[allSources.length - 1].srcset);
const srcset = allSources[allSources.length - 1].srcset
if (srcset) {
try {
return getSrcFromSrcset(srcset);
} catch (err) {
console.warn("Error parsing srcset", srcset);
}
}
}
}

Expand Down

0 comments on commit 65db0fb

Please sign in to comment.