Closed Bug 1046112 Opened 6 years ago Closed 4 years ago

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

Categories

(Toolkit :: Reader Mode, defect, P4)

ARM
Android
defect

Tracking

()

RESOLVED DUPLICATE of bug 1210366
Tracking Status
firefox31 --- affected
firefox32 --- affected
firefox33 --- affected
firefox34 --- affected
firefox35 --- affected
firefox36 --- affected
firefox37 --- affected
firefox38 --- affected
firefox39 --- affected
firefox40 --- affected
firefox41 --- affected
firefox42 --- affected
firefox43 --- affected

People

(Reporter: cos_flaviu, Unassigned)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file)

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.
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.
Component: Reader View → Reader Mode
Product: Firefox for Android → Toolkit
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.
Flags: needinfo?(flaviu.cos)
After tapping on the reader mode button a black page with a "Loading..." message appears for a second then the page reloads.
Flags: needinfo?(flaviu.cos)
(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.
(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.
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).
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...
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).
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...
Flags: needinfo?(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.
Assignee: nobody → margaret.leibovic
Flags: needinfo?(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.
Assignee: margaret.leibovic → nobody
Depends on: 1150174
Priority: -- → P4
Fixed by bug 1210366.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1210366
You need to log in before you can comment on or make changes to this bug.