Semantically, it is more correct to encode the fragment in the URL
instead of the URL-encoded `file` query parameter. This shouldn't matter
in practice, because `rewriteUrlClosure` in `chromecom.js` decodes the
`file` parameter and restores the fragment. However, as #16625 shows,
there was a case where this did not work as expected.
Note that these cases, which are all in older code, were found using the [`unicorn/no-for-loop`](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-for-loop.md) ESLint plugin rule.
However, note that I've opted not to enable this rule by default since there's still *some* cases where I do think that it makes sense to allow "regular" for-loops.
Please note that these changes were done automatically, using `gulp lint --fix`.
Given that the major version number was increased, there's a fair number of (primarily whitespace) changes; please see https://prettier.io/blog/2020/03/21/2.0.0.html
In order to reduce the size of these changes somewhat, this patch maintains the old "arrowParens" style for now (once mozilla-central updates Prettier we can simply choose the same formatting, assuming it will differ here).
Note that Prettier, purposely, has only limited [configuration options](https://prettier.io/docs/en/options.html). The configuration file is based on [the one in `mozilla central`](https://searchfox.org/mozilla-central/source/.prettierrc) with just a few additions (to avoid future breakage if the defaults ever changes).
Prettier is being used for a couple of reasons:
- To be consistent with `mozilla-central`, where Prettier is already in use across the tree.
- To ensure a *consistent* coding style everywhere, which is automatically enforced during linting (since Prettier is used as an ESLint plugin). This thus ends "all" formatting disussions once and for all, removing the need for review comments on most stylistic matters.
Many ESLint options are now redundant, and I've tried my best to remove all the now unnecessary options (but I may have missed some).
Note also that since Prettier considers the `printWidth` option as a guide, rather than a hard rule, this patch resorts to a small hack in the ESLint config to ensure that *comments* won't become too long.
*Please note:* This patch is generated automatically, by appending the `--fix` argument to the ESLint call used in the `gulp lint` task. It will thus require some additional clean-up, which will be done in a *separate* commit.
(On a more personal note, I'll readily admit that some of the changes Prettier makes are *extremely* ugly. However, in the name of consistency we'll probably have to live with that.)
Sometimes we also used `@return`, but `@returns` is what the JSDoc
documentation recommends. Even though `@return` works as an alias, it's
good to use the recommended syntax and to be consistent within the
project.
This rule is available from https://www.npmjs.com/package/eslint-plugin-mozilla, and is enforced in mozilla-central. Note that we have the necessary `Array`/`String` polyfills and that most cases have already been fixed, see PRs 9032 and 9434.
Merge ftp and file handler now their implementations are identical.
Remove redundant comment (the referenced Chrome bug has been fixed
a long time ago - https://crbug.com/302548 ).
http://eslint.org/docs/rules/comma-danglehttp://eslint.org/docs/rules/object-curly-spacing
Given that we currently have quite inconsistent object formatting, fixing this in in one big patch probably wouldn't be feasible (since I cannot imagine anyone wanting to review that); hence I've opted to try and do this piecewise instead.
Please note: This patch was created automatically, using the ESLint `--fix` command line option. In a couple of places this caused lines to become too long, and I've fixed those manually; please refer to the interdiff below for the only hand-edits in this patch.
```diff
diff --git a/extensions/firefox/content/PdfStreamConverter.jsm b/extensions/firefox/content/PdfStreamConverter.jsm
index ea91a71a..0d59dad1 100644
--- a/extensions/firefox/content/PdfStreamConverter.jsm
+++ b/extensions/firefox/content/PdfStreamConverter.jsm
@@ -773,7 +773,8 @@ class RequestListener {
response = function sendResponse(aResponse) {
try {
var listener = doc.createEvent("CustomEvent");
- let detail = Cu.cloneInto({ response: aResponse, }, doc.defaultView);
+ let detail = Cu.cloneInto({ response: aResponse, },
+ doc.defaultView);
listener.initCustomEvent("pdf.js.response", true, false, detail);
return message.dispatchEvent(listener);
} catch (e) {
```
Using `else` after `return` is not necessary, and can often lead to unnecessarily cluttered code. By using the `no-else-return` rule in ESLint we can avoid this pattern, see http://eslint.org/docs/rules/no-else-return.
A user encountered a response that looks like:
URL: some gibberish
Headers:
Content-Type: application/octet-stream
Content-Disposition: attachment; filename="something.pdf"
In the Chrome extension, the "attachment" content disposition is almost
always ignored (i.e. the PDF Viewer will try to view it anyway). So we
need to fall back to the Content-Disposition header if the URL check is
inconclusive.
The Chrome extension enforces that local files cannot be embedded in
non-local web pages. The previous check was too strict (because the
origin of a file:-URL is "null"), and prevented local PDF from being
viewed in local files).
This patch fixes that problem, by querying the actual tab URL via the
background page.
Steps to verify:
1. Create a HTML file: `<iframe src=test.pdf width=100% height=100%>`
2. Build and load the extension.
3. Allow file access to the extension at `chrome://extensions`
4. Open the HTML file from a file:// URL.
5. VERIFY: The extension should attempt to load the PDF file.
6. Now open the following (replace ID with the extension ID, which you
can find at `chrome://extensions`):
`data:text/html,<iframe src="chrome-extension://ID/file:///test.pdf">`
7. VERIFY: The next error should be displayed:
"Refused to load a local file in a non-local page for security reasons."
http://crbug.com/280464 has been resolved, so we can now use redirectUrl at
onHeadersReceived.
For backwards-compatibility, the code for the original method has not been
removed, and a feature detection script was added that detects whether the
desired feature is available.
The current stable Chromium version is 29. Since the critical
bug that prevented use of incognito:split has been fixed, and
incognito:split is actually used, it's safe to drop the code
that disables the extension in incognito mode.
( fixed bug = http:/crbug.com/224094 )
And use split incognito mode
Previous method:
- Rewrite content type to XHTML, followed by a content script
to cancel and replace the document with the viewer.
( https://github.com/mozilla/pdf.js/pull/3017 )
New method:
- Cancel loading of the document, followed by a redirect to the viewer
Disadvantage of new method:
- URLs are no longer "nice". This will be addressed by cherry-picking
a commit from the crx-using-streams-api branch.
Advantages of new method:
- Idle time is minimal. In some cases (with large documents),
it took too much time before the content script was activated.
During this period, the page looked blank, and the contents of
the PDF file were still retrieved and **discarded**.
With the new method, the idle time is minimal, because the request
is immediately cancelled.
- No FOUXEP (Flash of unhidden XML error page), because the XHTML
Content-Type hack is no longer used.
