872961 - Reader Mode controls do not work if the content is scrolled

Status: RESOLVED DUPLICATE of bug 877602

(Firefox for Android Graveyard :: Reader View, defect)

Description

[Adrian Tamas (:AdrianT)]

Aurora 23.0a2 2013-05-15/ Nightly 24.0a1 2013-05-15
HTC Desire Z (Android 2.3.3) / Samsung Galaxy S2 (Android 4.0.3)

Steps to reproduce:
1) Add a long page to the reading list (Make sure the content presented in the reading list can be scrolled)
2) Open the reading list item and scroll down the content
3) Tap on the content to open the reader mode controls and try to use any of them

Expected results:
The user can use the reader mode controls

Actual results:
The reader mode controls do not respond to taps until a repaint - rotating the device makes the controls usable again

Comment 1

[Aaron Train [:aaronmt]]

I'm not able to reproduce, HTC One (Android 4.1.2). Do you have an example URL? I tried this on a lengthy Wikipedia article and every time I accessed the reader toolbar after scrolling all it's corresponding buttons were respondent and working.

Comment 2

[Adrian Tamas (:AdrianT)]

URL: http://www.guardian.co.uk/world/2013/may/16/obama-fires-irs-head-tax-scandal

Screen videocapture: http://youtu.be/qXMoI5Q4jME

Try opening the reading list item in a new tab. This works for me every time if the page is loaded at the beginning. If I reload the content from a scrolled position I can't reproduce the issue.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Adrian - Try disabling the dynamic toolbar

Comment 4

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> Adrian - Try disabling the dynamic toolbar

Yeah, this is caused by the dynamic toolbar. I wonder if there's already a bug filed on this... seems like something that would apply to all webpages, not just about:reader.

Comment 5

[Adrian Tamas (:AdrianT)]

I can confirm that the issue is not reproducible if I disable the dynamic toolbar

Comment 6

[:Margaret Leibovic]

I can't reproduce this issue on the latest Nightly or Aurora. Can anyone else still reproduce?

On Nightly I do see some weirdness that makes the text style popup automatically disappear, but I think that's a separate issue to investigate.

Comment 7

[:Margaret Leibovic]

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

> On Nightly I do see some weirdness that makes the text style popup
> automatically disappear, but I think that's a separate issue to investigate.

I can't reproduce this on my build of the lastest mozilla-central, so maybe this was also fixed by something else.

Comment 8

[Adrian Tamas (:AdrianT)]

I can still reproduce this every time on the Samsung Galaxy R (Android 2.3.4) on Nightly 24.0a1

Steps:
1) Go to the Mozilla page at wikipedia.org and add it as a reading list item
2) Open a new tab and open the Mozilla page directly in reader mode
3) Scroll the page and then try to use the Reader Mode controls 

If the page is at the beginning when loaded and I scroll the page the controls are unusable. If I reload the page and then scroll it then I can use the controls.

Comment 9

[:Margaret Leibovic]

(In reply to Adrian Tamas from comment #8)
> I can still reproduce this every time on the Samsung Galaxy R (Android
> 2.3.4) on Nightly 24.0a1

Which build?

I can try with some more devices, but yesterday I couldn't reproduce on my Nexus 4.

Comment 10

[:Margaret Leibovic]

Aha, I was able to reproduce it now, I wasn't following the STR correctly. It works if you enter reader mode by toggling the reader mode icon in the toolbar, but it doesn't work if you open a page from the reading list.

Comment 11

[:Margaret Leibovic]

Investigating this, it appears that something is going wrong in the case where we load an article through AboutReader._loadFromURL, which calls Reader.parseDocumentFromURL. I suspected maybe something was wrong with loading the article from the cache, but the same problem is present even if the article isn't in the cache. I can also reproduce the bug if I force Reader.getArticleForTab to take the parseDocumentFromURL logic path.

I think figuring out what's different between these two code paths will help us figure out what's causing this bug. I'm not familiar with this Reader code, so I'm asking Lucas if he can take a look, too, in case he has any ideas.

(Also, Chris suggested I take a look at GeckoLayerClient to see if the fixed margins are set incorrectly, and logging the margins there, it looked like they were correct, so something else weird might be going on here.)

Comment 12

[Lucas Rocha (:lucasr)]

I'd force the content of the article to some known string to check if this is related to the content being presented in reader or not. In theory, loadFromURL and _loadFromTab should result on the same content being delivered to aboutReader.js.

My impression (without digging too deep here) is that this bug is related to dynamic toolbar. The reader toolbar is just a fixed element really...

Comment 13

[:Margaret Leibovic]

(In reply to Lucas Rocha (:lucasr) from comment #12)
> I'd force the content of the article to some known string to check if this
> is related to the content being presented in reader or not. In theory,
> loadFromURL and _loadFromTab should result on the same content being
> delivered to aboutReader.js.
> 
> My impression (without digging too deep here) is that this bug is related to
> dynamic toolbar. The reader toolbar is just a fixed element really...

Yeah, this is definitely a dynamic toolbar bug. I was just trying to see if I could figure out a reduced testcase that's tickling the problem.

When I was digging into this, I started to suspect that the problem is due to some delay in loading the content, since the problem only happens when loading from the cache or fetching the article over the network (not when we already have it in memory). So maybe some layout code isn't doing a re-calcuation it's supposed to do when the whole article gets added to the DOM.

I started to get frustrated by this bug, so I've been procrastinating looking into it more, but I'll try to get back into it today.

Comment 14

[:Margaret Leibovic]

Chris, I really think this is some weird layout issue I'm not qualified to fix, so I'd appreciate your input :)

For some reason, when loading the article from the cache or over the network (as opposed to from memory), the fixed position toolbar at the bottom of the page doesn't register clicks when the page is scrolled.

The callback function that loads the article is here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/aboutReader.js#513

I suspect that we're doing some layout calculations before this lazy-loaded content is added to the DOM, and then we're not updating it appropriately.

I'm also seeing this message frequently when tapping on the toolbar in the buggy case:
E/GeckoPanZoomController(21215): Received impossible touch end while in WAITING_LISTENERS

Comment 15

[:Margaret Leibovic]

Adding a needinfo? to kats as a reminder that he said he would take a look at this next week :)

Comment 16

[Kartikaya Gupta (email:[e-mail])]

I'm not able to reproduce the problem using the STR in comment 0 or comment 8. However, I can reproduce it if, after loading the reader mode page, I click on a link and then go back to the reader mode page and scroll. Weird. I will try to investigate what's going on here.

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Some more experimentation reveals that:
- this happens outside reader mode as well
- the entire toolbar-height area at the bottom of the screen doesn't respond to clicks

I traced through a bit with gdb and it looks like the frame that the touch event is targeted to ends up coming out as null and so the event is basically dropped. I'll need to dig around in the layout hit testing code a bit more to figure out why. I do also think this behaviour has changed recently because I can repro using comment 0 STR on my local build which is a week or two old but not on nightly. I can look at the changes that went in between those two to help isolate the code responsible.

Comment 18

[Kartikaya Gupta (email:[e-mail])]

So this bug is just a more specific case of bug 877602. I'm going to dupe it to that and investigate over there.