802881 - Wrong tab is used for getting background color

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Attachment: Use correct tab for getting background color

I discovered this when testing bug 799617.

STR:
1) Open Fennec and go to http://www.google.com
2) Open a new tab and go to http://www.izzyvideo.com/how-to-create-a-black-background-behind-your-subject/
3) Immediately switch back to the first tab
4) Wait 10 seconds
5) Switch to the second tab

Expected results:
The LayerView shows a dark background before the page is drawn

Actual results:
The LayerView shows a white background

Comment 1

[Brian Nicholson (:bnicholson)]

Note that these STR only work with the patches in bug 799617 applied.

Comment 2

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

Comment on attachment 672585 [details] [diff] [review]
Use correct tab for getting background color

Review of attachment 672585 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch!

::: mobile/android/chrome/content/browser.js
@@ +2809,5 @@
>          // checkerboard.) Right now we don't detect changes in the background color after this
>          // event fires; it's not clear that doing so is worth the effort.
>          var backgroundColor = null;
>          try {
> +          let { contentDocument, contentWindow } = this.browser;

Woah, this syntax is unfamiliar to me. I'm amazed that this does anything, let alone anything useful.

Comment 3

[Brian Nicholson (:bnicholson)]

(In reply to Kartikaya Gupta (:kats) from comment #2)
> ::: mobile/android/chrome/content/browser.js
> @@ +2809,5 @@
> >          // checkerboard.) Right now we don't detect changes in the background color after this
> >          // event fires; it's not clear that doing so is worth the effort.
> >          var backgroundColor = null;
> >          try {
> > +          let { contentDocument, contentWindow } = this.browser;
> 
> Woah, this syntax is unfamiliar to me. I'm amazed that this does anything,
> let alone anything useful.

Haha yeah, that caught me off-guard too when I saw it. Looks like it just extracts the properties of the object into local variables of the same name. You can also choose a different name for the local variables by using the syntax 'let { contentDocument: foo } = browser;', where the local variable foo is then set to browser.contentDocument. This syntax is marginally described in https://developer.mozilla.org/en-US/docs/JavaScript/New_in_JavaScript/1.7#Looping_across_values_in_an_array_of_objects (though that example uses 'in' instead of assignment).

Comment 4

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1a263da75260

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1a263da75260

Comment 6

[Andreea Pod]

Following the steps from description this is verified fixed.

Build: Firefox for Android 19.0a1 (2012-10-30)
Device: Samsung Galaxy Nexus
OS: Android 4.1.2