Closed Bug 756419 Opened 9 years ago Closed 9 years ago

White standalone image background flashes into view when switching tabs


(Toolkit :: Themes, defect)

15 Branch
Not set



Tracking Status
firefox15 + verified
firefox16 --- fixed


(Reporter: ferongr, Assigned: jaws)



(Keywords: regression, testcase, Whiteboard: [qa:see comment 17])


(1 file, 1 obsolete file)

Ever since bug 754133 landed, switching between standalone image tabs has become a disorienting procedure. With discarded decoded image data the system takes some time to re-decode the image. In that interval the white background is displayed instead of the image, resulting in brief a white flash.

Also, when slowly loading a large standalone image the experience is not ideal either.

Ideally the white background should only appear if there's a fully decoded image in view.
Blocks: 754133
Tracking this regression in 15 and bring Jared in to the discussion to see if we can forward fix this before 15 ships or whether we have to look at backing out bug 754133.
I'm recommending:
1) changing image background from white to black only when image is loading and is not completely rendered - this will prevent this flashing issue
2) changing background from dark gray to black - will have win in battery saving on mobiles, and dark grey isn't needed as we have now bug #754133 fixed. But this probably need a new bug.

What do you guys think about these ideas?
(In reply to Lukas Blakk [:lsblakk] from comment #3)
> Tracking this regression in 15 and bring Jared in to the discussion to see
> if we can forward fix this before 15 ships or whether we have to look at
> backing out bug 754133.

This isn't severe enough to back out bug 754133.
I agree with Dao.

It looks like this could be achieved by hooking in to the event. However, I don't think I'll have time to get this bug fixed before Aurora 15 merges to Beta 15.
Ever confirmed: true
Jared, assigning to you for now unless you can find someone else to take it. It's still early in the Beta cycle so we could take a fix in the coming weeks if you have time to prepare something.
Assignee: nobody → jaws
Attached patch Patch (obsolete) — Splinter Review
This patch sets and unsets the "decoded" class on the image when the image is decoded and discarded, respectively.

This *should* reduce instances of the white flashing, but I'm unable to reproduce it locally.

bholley: I flagged you for feedback due to the note in onStopDecode here <>

I think we'll be OK without the code to remove the observer inside of onStopDecode since the observer is still removed in ImageDocument::Destroy.
Attachment #643958 - Flags: review?(roc)
Attachment #643958 - Flags: feedback?(bobbyholley+bmo)
Comment on attachment 643958 [details] [diff] [review]

> ImageDocument::OnStopDecode(imgIRequest *aRequest,
>                             nsresult aStatus,
>                             const PRUnichar *aStatusArg)
> {
>   UpdateTitleAndCharset();
>-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(mImageContent);
>-  if (imageLoader) {
>-    mObservingImageLoader = false;
>-    imageLoader->RemoveObserver(this);
>-  }
>+  // Update the background-color of the image only after the
>+  // image has been decoded to prevent flashes of just the
>+  // background-color.
>+  mImageContent->SetAttr(kNameSpaceID_None, nsGkAtoms::_class,
>+                         NS_LITERAL_STRING("decoded"), false);

This is wrong, because OnStopDecode is called at _load_ time (once per image), rather than at decode time. The comment on MDN and in the source sort of explains this, but is unfortunately hampered by a very terrible typo (it should say that this method is a companion to OnStopRequest, not to OnStopDecode, which doesn't make sense).

If you want to hear about when an image finishes decoding, listing for OnStopContainer.
Attachment #643958 - Flags: feedback?(bobbyholley+bmo) → feedback-
Attached patch Patch v2Splinter Review
This patch uses OnStopContainer to get the proper decode notification.

Note that the observer is no longer removed in OnStopDecode, since it is still needed for OnDiscard. We could remove the observer in OnDiscard, but then we would need to re-add the observer when the image starts decoding again (but wouldn't get that notification since the necessary observer would be removed).
Attachment #643958 - Attachment is obsolete: true
Attachment #649087 - Flags: review?(roc)
Attachment #649087 - Flags: review?(bobbyholley+bmo)
Comment on attachment 649087 [details] [diff] [review]
Patch v2

r=bholley. Presumably roc's r+ means we're ok exposing .decoded naked/unprefixed?
Attachment #649087 - Flags: review?(bobbyholley+bmo) → review+
Yeah, we're ok unprefixed because this class is only applied on images in top-level synthetic documents.
Target Milestone: --- → mozilla17
Version: unspecified → 15 Branch
Closed: 9 years ago
Resolution: --- → FIXED
Comment on attachment 649087 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 754133
User impact if declined: users will see a brief flash of white before the image reloads when switching tabs
Testing completed (on m-c, etc.): landed on m-c on 8/9, tested locally
Risk to taking this patch (and alternatives if risky): minor risk, involves keeping an observer around longer than before, but no issues are expected
String or UUID changes made by this patch: none
Attachment #649087 - Flags: approval-mozilla-beta?
Attachment #649087 - Flags: approval-mozilla-aurora?
Attachment #649087 - Flags: approval-mozilla-beta?
Attachment #649087 - Flags: approval-mozilla-beta+
Attachment #649087 - Flags: approval-mozilla-aurora?
Attachment #649087 - Flags: approval-mozilla-aurora+
Is there a testcase QA can use to verify this is fixed?
Keywords: testcase-wanted
While not a real testcase, any sufficiently large image that doesn't decode instantly (e.g. )should suffice to verify.

1. Open and wait for the image to load.
2. Switch to another tab and wait for the image data to be discarded.
3. Switch back to the tab with the image.

Without the fix the white background intended to help with transparent images shows up until the image is decoded again and displayed.

With the fix, while the image is decoded you see no white background.
Thanks John. We'll try to verify it using that test.
Whiteboard: [qa:see comment 17]
Tested with and suggestions in comment 17 on 15b5 - Windows 7, Ubuntu and Mac OS.

No white background is displayed anymore and no other glitches spotted while switching tabs with large images.
Keywords: verifyme
QA Contact: virgil.dicu
Depends on: 788418
Depends on: 841547
You need to log in before you can comment on or make changes to this bug.