Closed Bug 1152817 Opened 9 years ago Closed 9 years ago

Reader View fails on article pages of the German Heise Online IT news site

Categories

(Toolkit :: Reader Mode, defect)

defect
Not set
normal
Points:
3

Tracking

()

RESOLVED FIXED
Iteration:
40.1 - 13 Apr
Tracking Status
firefox38 --- affected
firefox39 --- affected
firefox40 --- affected

People

(Reporter: MarcoZ, Assigned: Gijs)

References

()

Details

Attachments

(2 files)

Steps:
1. Visit, for example, http://www.heise.de/mac-and-i/meldung/1Password-fuer-Mac-generiert-Einmal-Passwoerter-2596987.html
2. Click the Enter Reader View button in the toolbar.

Expected: Article should appear reformatted.
Actual: Only heading appears, but no article text.

This can be reproduced with any article opened from http://www.heise.de/newsticker or http://www.heise.de/mac-and-i
The HTML, taken from View Source with Reader Mode with the above article open, looks like this (copied and pasted from NDA's virtual buffer):

1 <!DOCTYPE html>
2 <html>
3 
4 <head>
5   <meta content="text/html; charset=UTF-8" http-equiv="content-type" />
6   <meta name="viewport" content="width=device-width; user-scalable=0" />
7 
8   <link rel="stylesheet" href="chrome://global/skin/aboutReader.css" type="text/css"/>
9 
10   <script type="text/javascript;version=1.8" src="chrome://global/content/reader/aboutReader.js"></script>
11 </head>
12 
13 <body>
14   <div id="container" class="container">
15     <div id="reader-header" class="header">
16       <a id="reader-domain" class="domain"></a>
17       <div class="domain-border"></div>
18       <h1 id="reader-title"></h1>
19       <div id="reader-credits" class="credits"></div>
20     </div>
21 
22     <div id="reader-content" class="content">
23     </div>
24 
25     <div id="reader-message" class="message">
26     </div>
27 
28     <div id="reader-footer" class="footer">
29       <button id="remove-button" class="button remove-button"/>
30     </div>
31   </div>
32 
33   <ul id="reader-toolbar" class="toolbar">
34     <li><button id="close-button" class="button close-button"/></li>
35     <li><button id="share-button" class="button share-button"/></li>
36     <ul id="style-dropdown" class="dropdown">
37       <li><button class="dropdown-toggle button style-button"/></li>
38       <li id="reader-popup" class="dropdown-popup">
39         <div id="font-type-buttons"></div>
40         <hr></hr>
41         <div id="font-size-buttons">
42           <button id="font-size-minus" class="minus-button"/>
43           <button id="font-size-sample"/>
44           <button id="font-size-plus" class="plus-button"/>
45         </div>
46         <hr></hr>
47         <div id="color-scheme-buttons"></div>
48         <div class="dropdown-arrow"/>
49       </li>
50     </ul>
51     <li><button id="toggle-button" class="button toggle-button" hidden="true"/></li>
52     <li><button id="list-button" class="button list-button" hidden="true"/></li>
53   </ul>
54 
55 </body>
56 
57 </html>
58
What build are you using? Can you reproduce on a clean profile? What if you open the URL directly?
Flags: needinfo?(mzehe)
Using the 2015-04-08 nightly build for Windows 32-bit. And I can reproduce it with the link no matter how I open it.
Flags: needinfo?(mzehe)
This is working on 38 and not in nightly. I wonder why, and how long that situation will last with our regular updates... We should figure out when/why this broke.
OS: Windows 7 → All
Hardware: x86_64 → All
Not broken in Firefox Dev Edition 39.0a2. Works as expected there.
This is annoying. I'm trying to find a regression window and failing, because it seems to work on the builds I'm testing on Windows using mozregression. :-\
So this wfm in a clean profile on nightly, and doesn't work on one of my pre-existing nightly profiles. It continues to not work on earlier versions of Nightly with that pre-existing profile.
So it seems to be ad-related. Readability gives back:

<div id="readability-page-1" class="page">
    <div id="bannerzone">
        <p class="leaderboard">
            <noscript>&lt;div&gt;&lt;a href="http://ad-emea.doubleclick.net/N6514/jump/mac/mac-inhalt;sz=728x90,468x60;kw=1Password,Mac%20OS%20X,Passwort,Passwortmanager,Sicherheit,TOTP;tile=3;_YL_;ord=7840756744?"
                target="_blank"&gt;&lt;img alt="" src="http://ad-emea.doubleclick.net/N6514/ad/mac/mac-inhalt;sz=728x90,468x60;kw=1Password,Mac%20OS%20X,Passwort,Passwortmanager,Sicherheit,TOTP;tile=3;_YL_;ord=7840756744?"
                /&gt;&lt;/a&gt;&lt;/div&gt;</noscript>
        </p>
        <p class="skyscraper">
            <noscript>&lt;div&gt;&lt;a href="http://ad-emea.doubleclick.net/N6514/jump/mac/mac-inhalt;sz=120x600,120x800,160x600,160x800;kw=1Password,Mac%20OS%20X,Passwort,Passwortmanager,Sicherheit,TOTP;tile=4;_YL_;ord=7840756744?"
                target="_blank"&gt;&lt;img alt="" src="http://ad-emea.doubleclick.net/N6514/ad/mac/mac-inhalt;sz=120x600,120x800,160x600,160x800;kw=1Password,Mac%20OS%20X,Passwort,Passwortmanager,Sicherheit,TOTP;tile=4;_YL_;ord=7840756744?"
                /&gt;&lt;/a&gt;&lt;/div&gt;</noscript>
        </p>
    </div>
</div>


If you close the browser and reopen, it just XHRs the page which doesn't seem to have the banner (?) and it does the right thing. I'll attach the busted HTML in a second.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attached file HTML on which we choke
So this is because the ad includes noscript which has escaped HTML which includes a boatload of commas, which then ups the candidate score.

We should probably both nuke things that explicitly say "banner" or "skyscraper" in the className, and we can probably pretty safely nuke all <noscript> contents like we do with <script>. I'll do a PR for this.
Attached file Github PR
Attachment #8590405 - Flags: review?(margaret.leibovic)
Iteration: --- → 40.1 - 13 Apr
Points: --- → 3
Flags: qe-verify-
Flags: in-testsuite+
Flags: firefox-backlog+
Attachment #8590405 - Flags: review?(margaret.leibovic) → review+
We have earlier marked things fixed based on when things land in github, so I'm going to go ahead and do that here, too. m-c (and branches) landing will be visible in bug 1152022.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: