1160577 - Error message is no longer shown when a page isn't readable

Status: VERIFIED FIXED

(Toolkit :: Reader Mode, defect)

Description

[Matthew N. [:MattN]]

STR:
1) Load http://www.cbc.ca/mrd/episodes/season-4/
2) Click the Reader View icon in the URL bar

Actual result:
Where the readable text should appear it stays blank. The title of the page becomes "Failed to load article from page"

Expected result:
The error message in the page should be visible. The markup is there but it's hidden by CSS.

[Tracking Requested - why for this release]: Reader view regression

Comment 1

[:Margaret Leibovic]

This is a regression from bug 1154028, which hasn't been uplifted yet, so you should only see this in Nightly.

Comment 2

[:Margaret Leibovic]

The problem here is that I turned this:

<div id="reader-message" class="message"></div>

into this:

<div class="message">
  <div id="reader-message"></div>
</div>

We have JS that makes the message visible by setting a style on #reader-message, but the default CSS makes .message hidden, so changing the visibility of that child has no effect.

Patch coming.

Comment 3

[:Margaret Leibovic]

Attachment: MozReview Request: bz://1160577/margaret

/r/8067 - Bug 1160577 - Set styles on #reader-message div instead of wrapper div. r=MattN

Pull down this commit:

hg pull -r c7d75d9e4789c632e2b31b7d5f826d6e8cb8a162 https://reviewboard-hg.mozilla.org/gecko/

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8600512 [details]
MozReview Request: bz://1160577/margaret

https://reviewboard.mozilla.org/r/8065/#review6809

Ship It!

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/15e2e3b21f74

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15e2e3b21f74

Comment 7

[:Margaret Leibovic]

Comment on attachment 8600512 [details]
MozReview Request: bz://1160577/margaret

Approval Request Comment
[Feature/regressing bug #]: bug 1154028 
[User impact if declined]: error message will be invisible in reader view
[Describe test coverage new/current, TreeHerder]: landed on Nightly
[Risks and why]: low-risk style change restricted to reader view
[String/UUID change made/needed]: none

Comment 8

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 8600512 [details]
MozReview Request: bz://1160577/margaret

Approved for uplift to aurora. 38.0 has already been built so this can't make it into 38 without being part of a point release.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm hitting a merge conflict on the uplift to Aurora. Can you look into what's missing, Margaret?

Comment 10

[:Margaret Leibovic]

(In reply to Wes Kocher (:KWierso) from comment #9)
> I'm hitting a merge conflict on the uplift to Aurora. Can you look into
> what's missing, Margaret?

Oh, the reason is that we didn't uplift bug 1158194 (we don't need to do that for Android, it can just ride the trains). I landed a rebased patch:

https://hg.mozilla.org/releases/mozilla-aurora/rev/8051f796431e

This should also apply to 38.0.5.

Comment 11

[Andrei Vaida [:avaida]]

Confirmed that the "Failed to load article from page" error message is now shown on Nightly 40.0a1 (2015-05-07), using Windows 7 (x64) with this page [1] since the one from Comment 0 is no longer Reader View-compatible.

[1] http://store.makerbot.com/smartextruder

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8600512 [details]
MozReview Request: bz://1160577/margaret

Should be in 38.0.5 beta 1

Comment 13

[Petruta Horea [:phorea]]

The fix didn't reach Firefox 38.0.5 beta 1 build.

The issue is fixed on latest Developer Edition 2015-05-10 under all platforms.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/ad9164105253

Comment 15

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

The fix appears to have made it in 38.0.5 Beta 1 build 2.

Comment 16

[Petruta Horea [:phorea]]

Verified as fixed with Firefox 38.0.5 Beta 1 build 2 under Win 7 64-bit and Mac OS X 10.9.5 using the page from comment 11.

Comment 18

[:Margaret Leibovic]

Attachment: MozReview Request: Bug 1160577 - Set styles on #reader-message div instead of wrapper div. r=MattN