787416 - Reader Mode toolbar has layout issues

Status: VERIFIED FIXED

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

Description

[Cristian Nicolae (:xti)]

Attachment: screenshot

Firefox 18.0a1 (2012-08-31)
Device: Galaxy Note
OS: Android 4.0.4

Steps to reproduce:
1. Open Fennec
2. Open http://goo.gl/ycxWw in a tab where Desktop Site option is enabled
3. Tap on Reader Mode icon
4. Tap on the Layout Options button ("Aa")
* If the issue doesn't occur, go forward with the following steps:
5. Exit Reader Mode by tapping the back button
6. Repeat steps 3-4

Expected result:
There are no Reader Mode toolbar issues after steps 4 and 6.

Actual result:
Reader Mode toolbar has layout issues as you can see in the attached screenshot.

Comment 1

[Cristian Nicolae (:xti)]

Attachment: toolbar transparency

Sometimes, after I go to reader mode, when I tap on screen to trigger the reader mode toolbar, it seems to have a high transparency as you can see in this screenshot. However, the buttons are working properly.

Comment 2

[Lucas Rocha (:lucasr)]

Cristian, does this happen in Aurora and/or Beta?

Comment 3

[Chris Lord [:cwiiis]]

(In reply to Lucas Rocha (:lucasr) from comment #2)
> Cristian, does this happen in Aurora and/or Beta?

On IRC, he said it didn't happen in Aurora. This leads me to believe it may be a regression from bug 785333, but I hope not :( Will wait for the range.

Comment 4

[Cristian Nicolae (:xti)]

Attachment: Nightly 08/30

I am still not sure about the regression window here. 

On 08/31 is the first bad build where this issue occurs.

On 08/30 Nightly when I tap on Layout options, a black line is triggered just above the Reader Mode toolbar (see attached screenshot). Is it related to this issue? If yes, a possible regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=418955e7c3a9&tochange=706174d31a02

On 08/29 Nightly, the Reader Mode toolbar is broken and on the 08/28 it's not reproducible at all.

If the issue noticed on 30th is not related, then the possible regression range is: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=706174d31a02&tochange=fcc533f691e9

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to Cristian Nicolae (:xti) from comment #4)
> Created attachment 657826 [details]
> Nightly 08/30
> 
> I am still not sure about the regression window here. 
> 
> On 08/31 is the first bad build where this issue occurs.
> 
> On 08/30 Nightly when I tap on Layout options, a black line is triggered
> just above the Reader Mode toolbar (see attached screenshot). Is it related
> to this issue? If yes, a possible regression range is:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=418955e7c3a9&tochange=706174d31a02
> 
> On 08/29 Nightly, the Reader Mode toolbar is broken and on the 08/28 it's
> not reproducible at all.
> 
> If the issue noticed on 30th is not related, then the possible regression
> range is:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=706174d31a02&tochange=fcc533f691e9

I've seen the black line artifact before layout issues started happening in Nightly so I think they are unrelated.

Comment 6

[Cristian Nicolae (:xti)]

(In reply to Lucas Rocha (:lucasr) from comment #5)
> 
> I've seen the black line artifact before layout issues started happening in
> Nightly so I think they are unrelated.

Then I will narrow down the 2nd regression range using the tinderbox inbound builds. In this case the build from 08/30 is the last known good build and the one from 08/31 is the first bad build.

Comment 7

[Cristian Nicolae (:xti)]

Good build:
http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android/1346354503/

Bad build:
http://ftp.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android/1346355403/

Regression range:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ce60fb4481d5&tochange=d04edff1ae41

--
Device: Galaxy Note
OS: Android 4.0.4

Comment 9

[Benoit Girard (:BenWa)]

I found the problem. My patch in bug 751916 assumes that changing the format of a layer requires us to recreate/invalidate it and this isn't the case. Investigating if it's an assumption we want or if I need to fix my code to copy the image data when changing the format.

Comment 10

[Benoit Girard (:BenWa)]

Attachment: patch

Follow up to my previous patch. If we change format we invalidate the whole layer instead of trying to support it tile by tile.

Comment 11

[Benoit Girard (:BenWa)]

Comment on attachment 658166 [details] [diff] [review]
patch

https://hg.mozilla.org/integration/mozilla-inbound/rev/9619aab9a050

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9619aab9a050

Comment 13

[Cristian Nicolae (:xti)]

This issue is fixed on the latest Nightly. Closing bug as verified fixed on:

Firefox 18.0a1 (2012-09-10)
Device: Galaxy Note
OS: Android 4.0.4