1046112 - Reader mode icon is displayed on m.youtube.com page

Status: RESOLVED DUPLICATE of bug 1210366

(Toolkit :: Reader Mode, defect, P4)

Description

[Flaviu Cos]

Attachment: reader mode icon on youtube.png

Environment: 
Device: Asus Transformer Tab (Android 4.2.1);
Build: Nightly 34.0a1 (2014-07-30);

Steps to reproduce:
1. Go to m.youtube.com;
2. Choose any video from the list;
3. Refresh the page.

Expected result:
Only the helper app icon is displayed in the url bar.

Actual result:
The helper app and the reader mode icons are displayed in the url bar.

Notes:
Please check the attached screenshot.

Comment 1

[Aaron Train [:aaronmt]]

It would be helpful to produce a minimal test-case on YouTube trying to figure out why it's being detected. YouTube is a very complex site, so narrowing this down would probably be helpful on identifying detection issues on similar sites.

Comment 2

[Justin Dolske [:Dolske]]

What actually gets displayed if the user activates reader view on Youtube?

Can't test on Desktop because I get no reader mode icon for YouTube videos, and m.youtube.com redirects back to the non-mobile site.

Comment 3

[Flaviu Cos]

After tapping on the reader mode button a black page with a "Loading..." message appears for a second then the page reloads.

Comment 4

[:Margaret Leibovic]

(In reply to Flaviu Cos, QA [:flaviu] from comment #3)
> After tapping on the reader mode button a black page with a "Loading..."
> message appears for a second then the page reloads.

Does that happen on Nightly? That sounds like bug 1128757.

With the fix for that bug, we shouldn't be showing a reader button unless there's actual article content to display.

Comment 5

[Flaviu Cos]

(In reply to :Margaret Leibovic from comment #4)

> Does that happen on Nightly? That sounds like bug 1128757.
> 
> With the fix for that bug, we shouldn't be showing a reader button unless
> there's actual article content to display.

Tested on latest nightly (2015-03-13) and the reader mode button is still displayed on youtube sites, but clicking on it it will display the youtube page in reader mode.

Comment 6

[:Margaret Leibovic]

I'm also seeing us parse content for some desktop youtube pages, e.g.:
https://www.youtube.com/watch?v=XAHprLW48no

However, this isn't really that useful without the video, so maybe we should try to avoid showing the button in these cases (like part of bug 1143844).

Comment 7

[:Gijs (he/him)]

With the latest changes on fx-team, I no longer see a reader mode icon on the youtube page in question. I can't check mobile because I get redirected...

Comment 8

[Flaviu Cos]

The reader mode icon is still present on mobile version of youtube.
Tested on latest nightly (2015-03-23) using Asus Transformer Tab (Android 4.2.1).

Comment 9

[:Gijs (he/him)]

Margaret, maybe we can use this bug to copy the new algorithm we use to determine whether we show the icon on desktop to be used on mobile as well? Bonus points if we find some common location to put it...

Comment 10

[:Margaret Leibovic]

(In reply to :Gijs Kruitbosch from comment #9)
> Margaret, maybe we can use this bug to copy the new algorithm we use to
> determine whether we show the icon on desktop to be used on mobile as well?
> Bonus points if we find some common location to put it...

It's already in ReaderMode.jsm, so it's shared :D

We've been discussing implementing this new button logic on mobile, but we wanted to hold off until we see how well it works on desktop. However, it seems pretty good to me, and the memory/perf improvements would be worth it.

At the very least, we can try landing it on Nightly, and just not uplift it if we think it's too risky.

Comment 11

[:Margaret Leibovic]

(In reply to :Margaret Leibovic from comment #10)
> (In reply to :Gijs Kruitbosch from comment #9)
> > Margaret, maybe we can use this bug to copy the new algorithm we use to
> > determine whether we show the icon on desktop to be used on mobile as well?
> > Bonus points if we find some common location to put it...
> 
> It's already in ReaderMode.jsm, so it's shared :D
> 
> We've been discussing implementing this new button logic on mobile, but we
> wanted to hold off until we see how well it works on desktop. However, it
> seems pretty good to me, and the memory/perf improvements would be worth it.
> 
> At the very least, we can try landing it on Nightly, and just not uplift it
> if we think it's too risky.

I filed bug 1150174 about this, since I don't want to cause confusion by changing the purpose of this bug.

Comment 12

[:Gijs (he/him)]

Fixed by bug 1210366.