Closed Bug 1377144 Opened 2 years ago Closed 2 years ago
Reader Mode: text-colored dot in the bottom right of the screen
With Firefox for Android nightly (56.0a1 2017-06-28), I am able to reproduce an issue with 2 different websites: - about:reader?url=http%3A%2F%2Fmashable.france24.com%2Ftech-business%2F20170627-monnaie-virtuelle-ethereum-consommation-electricite - about:reader?url=http%3A%2F%2Fdussutou.free.fr%2Fpublication_audrey_dussutour.html The issue appear as a text-colored dot, at the bottom right of the screen, and it does not move while scrolling. It appears both in portrait and landscape views.
I see the same thing on two different devices, using nytimes.com and washingtonpost.com.
I reported this in bug 1376419.
I do like me my ReaderMode .... This seems to regress from Bug 1291515: https://hg.mozilla.org/mozilla-central/rev/9ed72cb74889 Re-setting |pref("layout.css.scoped-style.enabled", true);| works as a work around for now.
Regression window performed: Last good revision: d50abca6521baeae8ac6b07ddf843d63a1aa5f84 (2017-06-26) First bad revision: 8f80d594c08d5c7a112e5d4b9eb44ffca717eb7b (2017-06-27)
This bug just showed up in beta 55.0-2015498601. beta 55.0-2015496769 is ok. The config fix works for the new beta.
[Tracking Requested - why for this release]: Reader mode config button broken on Android.
Hi Tim, could you find someone to look at this? I'm not sure where the code is that uses <style scoped> (the disabling of which got uplifted to beta) to add the in-page UI for reader mode.
Look like we'll need to partially revert bug 1154028. http://searchfox.org/mozilla-central/rev/b7e531410ebc1c7d74a78e86a894e69608db318c/toolkit/components/reader/content/aboutReader.html#16 Gijs, assuming bug 1154028 comment 1 is still correct, this can be fixed by prepending class names both content and controls on all selectors right? Please suggest any alternatives if you have any.
Flags: needinfo?(timdream) → needinfo?(gijskruitbosch+bugs)
If we want a quick fix, we could just allow <style scoped> in about:reader?xxx pages for now. (Stylo won't be enabled for about: pages other than about:blank initially, so that won't affect us.)
regression from a recent uplift to beta55, tracking.
FWIW unless there's a low-risk fix for reader mode in the next few hours I'll probably ask for part 2 of bug 1291515 to be backed out of beta so the regression is fixed in today's build.
Comment 13 is pretty low risk. Let me work that one up.
Attachment #8884737 - Flags: review?(xidorn+moz)
The patch is untested since I don't have a local environment set up for building Fennec. I've pushed a try run so hopefully I can grab a build from that to test: https://treeherder.mozilla.org/#/jobs?repo=try&revision=520beb2d9b0a4436aa6465cb3ad4e6a018042628
Attachment #8884737 - Flags: review?(xidorn+moz) → review?(emilio+bugs)
Comment on attachment 8884737 [details] Bug 1377144 - Allow <style scoped> in about:reader documents. https://reviewboard.mozilla.org/r/155620/#review160618 ::: commit-message-39f79:1 (Diff revision 1) > +Bug 1377144 - Allow <scoped style> in about:reader documents. nit: `<style scoped>`. ::: dom/base/nsDocument.cpp:13485 (Diff revision 1) > mIsThirdParty.emplace(false); > return mIsThirdParty.value(); > } > > +static bool > +IsAboutReader(nsIURI* aURI) nit: Perhaps worth leaving a comment about why we have to whitelist about:reader URIs?
Attachment #8884737 - Flags: review?(emilio+bugs) → review+
:maliu confirmed that the patch fixes the regression.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/3d48958204f3 Allow <style scoped> in about:reader documents. r=emilio
Comment on attachment 8884737 [details] Bug 1377144 - Allow <style scoped> in about:reader documents. Approval Request Comment [Feature/Bug causing the regression]: bug 1291515 [User impact if declined]: potential odd styling in Reader Mode on Android [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: OK: 1. Open this bug's URL field. 2. Notice a circle that shouldn't be present (as in this bug's screen shot attachment), if the fix didn't work. If the fix did work no circle should be present. [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: it does a simple URL check on the document and just re-enables <style scoped> for about:reader documents [String changes made/needed]: none
Attachment #8884737 - Flags: approval-mozilla-beta?
(In reply to Tim Guan-tin Chien [:timdream] (please needinfo) from comment #12) > Look like we'll need to partially revert bug 1154028. > > http://searchfox.org/mozilla-central/rev/ > b7e531410ebc1c7d74a78e86a894e69608db318c/toolkit/components/reader/content/ > aboutReader.html#16 > > Gijs, assuming bug 1154028 comment 1 is still correct, this can be fixed by > prepending class names both content and controls on all selectors right? > Please suggest any alternatives if you have any. It wouldn't really be a complete fix (unless we go overboard with child selectors to fix depth-from-document-root) because we know that we aren't currently perfect at stripping out styles and classes from readerized web content. That's not the end of the world, but really it points to the fact that the correct solution to both this and several other reader mode problems we have is to sandbox-iframe the readerized content. This is bug 1204818. It's not going to be trivial to do, though srcdoc should make it easier now than it used to be and so it shouldn't be rocket science, either. If we can go with a stopgap fix (which it looks like we're doing) for now and then prioritize the sandboxing bug that would be the best way forward, IMO.
Comment on attachment 8884737 [details] Bug 1377144 - Allow <style scoped> in about:reader documents. fix a regression in fennec reader mode introduced in 55.0b7 Should be in 55.0b8
Attachment #8884737 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed on Beta 55.0b8 (2017-11-07). Devices: -HTC Nexus 9 (Android 7.1.1) -Prestigio Grace X5 (Android 4.4.2) -HTC 10 (Android 6.0.1) -LG G4 (Android 5.1)
You need to log in before you can comment on or make changes to this bug.