Skip to content

Commit

Permalink
Revert change to routing to include legacy download info again
Browse files Browse the repository at this point in the history
This was breaking Chrome
  • Loading branch information
gyng committed Jul 26, 2018
1 parent 18abbae commit 38d469d
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 20 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 3.0.0

* Fix Chrome rules matching against `_`, now matches against special characters instead
* Fix routing failing in some cases (regression) (#80)
* Fix last used access key not working at all (Chrome)

# 2.7.3

* Fix accesskeys not appearing in Chrome (regression) (#79)
Expand Down
2 changes: 1 addition & 1 deletion manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"manifest_version": 2,
"name": "__MSG_extensionName__",
"description": "__MSG_extensionDescription__",
"version": "2.7.3",
"version": "2.7.4",
"default_locale": "en",

"applications": {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "save-in",
"version": "2.7.3",
"version": "2.7.4",
"license": "MIT",
"scripts": {
"build": "env -u WEB_EXT_API_KEY -u WEB_EXT_API_SECRET web-ext build --overwrite-dest -i test docs yarn.lock yarn-error.log",
Expand Down
7 changes: 5 additions & 2 deletions src/download.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ const Download = {
return null;
}

return Router.matchRules(filenamePatterns, state.info);
return Router.matchRules(
filenamePatterns,
state.info.legacyDownloadInfo,
state.info
);
},

renameAndDownload: state => {
Expand All @@ -67,7 +71,6 @@ const Download = {
});

state.path = Variable.applyVariables(state.path, state.info);

// FIXME: Fix router params for new path struct
const routeMatches = Download.getRoutingMatches(state);
if (routeMatches) {
Expand Down
8 changes: 5 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ window.init = () => {
}

if (options.enableLastLocation) {
const lastUsedTitle = lastUsedPath || browser.i18n.getMessage("contextMenuLastUsed");
const lastUsedMenuOptions = {
id: `save-in-_-_-last-used`,
title: lastUsedPath || browser.i18n.getMessage("contextMenuLastUsed"),
title: setAccesskey(lastUsedTitle, options.keyLastUsed),
enabled: lastUsedPath ? true : false, // eslint-disable-line
contexts: media,
parentId: "save-in-_-_-root"
Expand Down Expand Up @@ -280,13 +281,14 @@ browser.contextMenus.onClicked.addListener(info => {
now: new Date(),
pageUrl: info.pageUrl,
selectionText: info.selectionText,
sourceUrl: info.srcUrl,
sourceUrl: info.srcUrl || info.url,
url, // Changes based off context
suggestedFilename, // wip: rename
context: downloadType,
menuIndex,
comment,
modifiers: info.modifiers
modifiers: info.modifiers,
legacyDownloadInfo: info // wip, remove
};

// keeps track of state of the final path
Expand Down
3 changes: 2 additions & 1 deletion src/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ const Menus = {
context: DOWNLOAD_TYPES.TAB,
menuIndex: null,
comment: null,
modifiers: info.modifiers
modifiers: info.modifiers,
legacyDownloadInfo: info // wip, remove
};

// keeps track of state of the final path
Expand Down
6 changes: 5 additions & 1 deletion src/option.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,12 @@ const OptionsManagement = {
// filenamePatterns: options.filenamePatterns
// }
// };

const newInfo = Object.assign({}, state.info, {
filenamePatterns: options.filenamePatterns
filenamePatterns: options.filenamePatterns,
// Chrome hack for filename: Chrome replaces special characters with `_`
// This mutates(?) the last object and ruins it
filename: state.info.initialFilename || state.info.filename
});
const last = Object.assign({}, state, { info: newInfo });

Expand Down
8 changes: 0 additions & 8 deletions src/options/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,6 @@ <h4>Moving downloads into different directories</h4>

<p>__MSG_o_lRoutingInstruction__</p>

<p class="chrome-only">CHROME USERS: Special characters in the filename (eg.
<code>:</code> or
<code>/</code>) are pre-replaced by Chrome. Match these characters using a
<code>-</code> instead. For example, use
<code>(.*)-large</code> to match filenames instead of
<code>(.*):large</code>.
</p>

<textarea type="textarea" id="filenamePatterns" rows=7 placeholder="Optional" spellcheck="false"></textarea>
<div class="error-notification" id="error-filenamePatterns">errors</div>
<div style="display: flex; max-width: 100%; align-items: center; flex-wrap: wrap;">
Expand Down
5 changes: 2 additions & 3 deletions src/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,9 @@ const Router = {
return destination;
},

matchRules: (rules, info) => {
matchRules: (rules, info, rest) => {
for (let i = 0; i < rules.length; i += 1) {
// TODO: Legacy, combine both info into single info object
const result = Router.matchRule(rules[i], info, info);
const result = Router.matchRule(rules[i], info, rest);
if (result) {
return result;
}
Expand Down
125 changes: 125 additions & 0 deletions test/fixtures/clickInfo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
const firefoxInfo = {
currentTab: {
id: 2,
index: 1,
windowId: 5,
highlighted: true,
active: true,
pinned: false,
status: "complete",
discarded: false,
incognito: false,
width: 1268,
height: 854,
lastAccessed: 1532606200506,
audible: false,
mutedInfo: {
muted: false
},
isArticle: false,
isInReaderMode: false,
url: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
title: "Di6uEBuVsAEYVBw.jpg:orig (JPEG Image, 25 × 24 pixels)",
favIconUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig"
},
linkText: "",
now: "2018-07-26T11:56:42.642Z",
pageUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
sourceUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
url: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
suggestedFilename: null,
context: "MEDIA",
menuIndex: "1",
comment: "",
modifiers: [],
legacyDownloadInfo: {
menuItemId: "save-in-1--.",
editable: false,
parentMenuItemId: "save-in-_-_-root",
mediaType: "image",
linkText: "",
linkUrl: "",
srcUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
pageUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
frameUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
frameId: 0,
modifiers: []
},
naiveFilename: "Di6uEBuVsAEYVBw.jpg:orig",
filename: "Di6uEBuVsAEYVBw.jpg:orig",
initialFilename: "Di6uEBuVsAEYVBw.jpg:orig",
filenamePatterns: [
[
{
name: "filename",
value: {},
type: "MATCHER"
},
{
name: "sourceurl",
value: {},
type: "MATCHER"
},
{
name: "capture",
value: "filename",
type: "CAPTURE"
},
{
name: "into",
value: ":$1:",
type: "DESTINATION"
}
]
]
};

const chromeInfo = {
currentTab: {
active: true,
audible: false,
autoDiscardable: true,
discarded: false,
height: 748,
highlighted: true,
id: 1069,
incognito: false,
index: 1,
mutedInfo: {
muted: false
},
openerTabId: 1060,
pinned: false,
selected: true,
status: "loading",
title: "Di6uEBuVsAEYVBw.jpg:orig (25×24)",
url: "chrome://newtab/",
width: 1236,
windowId: 1059
},
now: "2018-07-26T12:07:55.529Z",
pageUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
sourceUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
url: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
suggestedFilename: null,
context: "MEDIA",
menuIndex: "1",
comment: "",
legacyDownloadInfo: {
editable: false,
frameId: 0,
mediaType: "image",
menuItemId: "save-in-1--.",
pageUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig",
parentMenuItemId: "save-in-_-_-root",
srcUrl: "https://pbs.twimg.com/media/Di6uEBuVsAEYVBw.jpg:orig"
},
naiveFilename: "Di6uEBuVsAEYVBw.jpg:orig",
filename: "Di6uEBuVsAEYVBw.jpg_orig",
initialFilename: "Di6uEBuVsAEYVBw.jpg:orig"
};

module.exports = {
chromeInfo,
firefoxInfo
};
49 changes: 49 additions & 0 deletions test/router.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const router = require("../src/router.js");
const downloads = require("../src/download.js");
const constants = require("../src/constants.js");
const fixtures = require("./fixtures/clickInfo");

describe("filename rewrite and routing", () => {
const info = {
Expand Down Expand Up @@ -176,4 +177,52 @@ describe("filename rewrite and routing", () => {
expect(match).toBe(null);
});
});

describe("browser context menu click integration", () => {
beforeAll(() => {
global.RULE_TYPES = constants.RULE_TYPES;
});

test("parses Firefox clicks", () => {
const twitterRulesFirefox = router.parseRules(
[
"filename: (.*)(:|-|=)(large|small|medium|thumb|orig)",
"sourceurl: pbs.twimg.com",
"capture: filename",
"into: :$1:"
].join("\n")
);

const clickInfo = fixtures.firefoxInfo;

const matched = router.matchRules(
twitterRulesFirefox,
clickInfo.legacyDownloadInfo,
clickInfo
);

expect(matched).toBe("Di6uEBuVsAEYVBw.jpg");
});

test("parses Chrome clicks", () => {
const twitterRulesChrome = router.parseRules(
[
"filename: (.*)_(large|small|medium|thumb|orig)",
"sourceurl: pbs.twimg.com",
"capture: filename",
"into: :$1:"
].join("\n")
);

const clickInfo = fixtures.chromeInfo;

const matched = router.matchRules(
twitterRulesChrome,
clickInfo.legacyDownloadInfo,
clickInfo
);

expect(matched).toBe("Di6uEBuVsAEYVBw.jpg");
});
});
});

0 comments on commit 38d469d

Please sign in to comment.