Wrong tab is used for getting background color

VERIFIED FIXED in Firefox 19

Status

()

defect
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: bnicholson, Assigned: bnicholson)

Tracking

unspecified
Firefox 19
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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
Attachment #672585 - Flags: review?(bugmail.mozilla)
Assignee

Comment 1

7 years ago
Note that these STR only work with the patches in bug 799617 applied.
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.
Attachment #672585 - Flags: review?(bugmail.mozilla) → review+
Assignee

Comment 3

7 years ago
(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).
https://hg.mozilla.org/mozilla-central/rev/1a263da75260
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19

Comment 6

7 years ago
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.