Regression : An added "PDF Viewer" extension doesn't get applied to certain PDF pages.
Reported by
avsha...@etouch.net,
Oct 20 2017
|
|||||||||
Issue descriptionChrome version : 64.0.3245.0 (Official Build) 140f8ce04cc4ee5d4b3ca2eb83888d2452a1f43b-refs/heads/master@{#510270} 32/64 bit OS : Windows(7,8,10), Linux(14.04 LTS), Mac(10.12.6) Test URL 1 : https://chrome.google.com/webstore/detail/pdf-viewer/oemmndcbldboiebfnladdacbdfmadadm?utm_source=chrome-ntp-icon Test URL 2 : https://msu.edu/~urban/sme865/resources/embedded_pdf.html What steps will reproduce the problem? 1. Launch chrome, navigate to test URL 1 and install an "PDF Viewer" extension. 2. Now open NTP and navigate to test URL 2 (let the page load completely). 3. Scroll down the page and observe the PDF. Actual Result : The "PDF Viewer" extension doesn't get applied to certain PDF pages. Expected Result : Added "PDF Viewer" extension should get applied to all PDF pages. This is a regression issue broken in ‘M-64’ and using the per-revision bisect providing the bisect results, Good build : 64.0.3241.0 (Revision : 508935) Bad build : 64.0.3242.0 (Revision : 509211) You are probably looking for a change made after 509093 (known good), but no later than 509095 (first known bad). CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/0da0da31394b2137d63e0f9697de13f2453ff833..e4488e203ff33223f228dd2359383d2085a76c07 Suspect : https://chromium.googlesource.com/chromium/src/+/e4488e203ff33223f228dd2359383d2085a76c07 Note : Above issue can also be reproduced on another URL - https://pdfobject.com/ @sky : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.
,
Oct 20 2017
I reverted my patch locally and it didn't impact this (nor would I have expected it to). I suspect you have the regression range wrong. I've passing to wjmaclean, maybe he has an idea as to change might have broken this.
,
Oct 20 2017
+ekaramad@ who knows about PDF loading, and +creis@ since this looks like a possible navigation issue?
,
Oct 20 2017
I don't think it is MimeHandlerView related. I think the extension redirects the chrome-extension://kPdfExtensionId/index.html?pdf to its own url [1]. So maybe the webRequest.onBeforeRequest is failing? 1- https://github.com/mozilla/pdf.js/blob/master/extensions/chromium/extension-router.js
,
Oct 20 2017
(Yet again) Not in the internal PDF viewer -> Internals>Plugins>PDF is the wrong label.
,
Oct 20 2017
Maybe there was a webRequest change recently? Devlin, could you help triage? I'm requesting another bisect since the CLs in the regression range don't seem right. Could be related to a field trial. (I've verified it's not the --site-per-process trial which is going on this week, since I can repro the bug on 64.0.3245.0 with or without that flag.)
,
Oct 20 2017
I'm not aware of any webRequest changes recently. Presumably this wouldn't be PlzNavigate related, either? karandeepb@, any ideas?
,
Oct 20 2017
Again, not really aware of any webRequest changes. Will wait for another bisect.
,
Oct 23 2017
With response to C#6, Performed another bisect and got different results: CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/e4488e203ff33223f228dd2359383d2085a76c07..30168b99604c48a32828f26b5acb2674beea1a5d Suspect : https://chromium.googlesource.com/chromium/src/+/30168b99604c48a32828f26b5acb2674beea1a5d @Manish : Could you please look into the issue.
,
Oct 23 2017
I reverted my change and tried out the extension, it is still broken. I looked at the extension's code: It injects one CSS file into the content script [1], and while this could have been the problem, it doesn't seem like this is the case. [1]: https://github.com/mozilla/pdf.js/blob/d9f90d595d0eb64875e9d3ed252ae0d0c2aca2bf/extensions/chromium/manifest.json#L28
,
Oct 23 2017
Well hold on ... I may have made a mistake, it does seem to work after I revert my change. I'm not sure if this is intermittent or what. Let me investigate.
,
Oct 23 2017
This is definitely a regression caused by 30168b9 The extension uses the "animationstart" event to detect the presence of embedded PDFs. Animations in injected style sheets are no longer working.
,
Oct 23 2017
As per c#12, un-assigning myself and assigning to nainar@ for further triage (Can't assign to m.jethani@ and rune@ doesn't seem to be in use any more).
,
Oct 23 2017
Yup, rune@opera.com isn't in use anymore. m.jethani@ this issue is on Canary so feel free to take some time to look into the issue. Ideally we should have this issue resolved in time for branch which is late November. If you want to initiate a revert let me know here and I can initiate it for you.
,
Oct 24 2017
I've uploaded a patch: https://crrev.com/c/735263
,
Oct 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bed9cb9d56adac6f50313b0e620a1aa2f0d871ed commit bed9cb9d56adac6f50313b0e620a1aa2f0d871ed Author: Manish Jethani <m.jethani@eyeo.com> Date: Sat Oct 28 23:02:07 2017 Support @keyframes rules in user style sheets Previously @keyframes rules in style sheets injected by extensions were managed in ScopedStyleResolver along with other author rules. With crrev.com/c/641294 we started treating these injected style sheets as user style sheets and moved the management of such rules to StyleEngine. This patch fixes a regression caused by this change whereby StyleResolver is no longer able to find @keyframes rules from these style sheets. StyleEngine now maintains a map of animation names to their corresponding @keyframes rules found in user style sheets. If StyleResolver is unable to find a given @keyframes rule among its author rules, it looks it up in StyleEngine next. BUG= 632009 , 776661 Change-Id: I90462a0721cc75da14ee041c1a70736e4df99fb8 Reviewed-on: https://chromium-review.googlesource.com/735263 Reviewed-by: nainar <nainar@chromium.org> Commit-Queue: nainar <nainar@chromium.org> Cr-Commit-Position: refs/heads/master@{#512411} [modify] https://crrev.com/bed9cb9d56adac6f50313b0e620a1aa2f0d871ed/third_party/WebKit/Source/core/css/StyleEngine.cpp [modify] https://crrev.com/bed9cb9d56adac6f50313b0e620a1aa2f0d871ed/third_party/WebKit/Source/core/css/StyleEngine.h [modify] https://crrev.com/bed9cb9d56adac6f50313b0e620a1aa2f0d871ed/third_party/WebKit/Source/core/css/StyleEngineTest.cpp [modify] https://crrev.com/bed9cb9d56adac6f50313b0e620a1aa2f0d871ed/third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp
,
Nov 1 2017
I followed the STR from the report and can confirm that my PDF Viewer extension works as expected in 64.0.3256.0. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by ranjitkan@chromium.org
, Oct 20 2017