The default bug view has changed. See this FAQ.

White standalone image background flashes into view when switching tabs

RESOLVED FIXED in Firefox 15

Status

()

Toolkit
Themes
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: ferongr, Assigned: jaws)

Tracking

({regression, testcase})

15 Branch
mozilla17
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox15+ verified, firefox16 fixed)

Details

(Whiteboard: [qa:see comment 17])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 754133
Confirmed. It's really annoying!
Gentle ping
and what are we going to do with this bug?
tracking-firefox15: --- → ?
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.
tracking-firefox15: ? → +
Keywords: regression
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 https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIDecoderObserver#onStopDecode() event. However, I don't think I'll have time to get this bug fixed before Aurora 15 merges to Beta 15.
Status: UNCONFIRMED → NEW
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
Created attachment 643958 [details] [diff] [review]
Patch

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 <https://developer.mozilla.org/en/XPCOM_Interface_Reference/imgIDecoderObserver#onStopDecode%28%29>

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)
Attachment #643958 - Flags: review?(roc) → review+
Comment on attachment 643958 [details] [diff] [review]
Patch


> 
> NS_IMETHODIMP
> 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-
Created attachment 649087 [details] [diff] [review]
Patch v2

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)
Attachment #649087 - Flags: review?(roc) → review+
Status: NEW → ASSIGNED
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.

https://hg.mozilla.org/integration/mozilla-inbound/rev/35ac9dfcef9b
Target Milestone: --- → mozilla17
Version: unspecified → 15 Branch
https://hg.mozilla.org/mozilla-central/rev/35ac9dfcef9b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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+
https://hg.mozilla.org/releases/mozilla-beta/rev/6bc2cad5a3b4
https://hg.mozilla.org/releases/mozilla-aurora/rev/7bd72ea6ab99
status-firefox15: --- → fixed
status-firefox16: --- → fixed
Is there a testcase QA can use to verify this is fixed?
Keywords: testcase-wanted
(Reporter)

Comment 17

5 years ago
While not a real testcase, any sufficiently large image that doesn't decode instantly (e.g. http://kurtjohnson.net/Photos/2006.12.30.MP_pano.jpg )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.
Keywords: testcase-wanted → testcase, verifyme
Whiteboard: [qa:see comment 17]
Tested with http://www.free-pictures-photos.com/ 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.
status-firefox15: fixed → verified
Keywords: verifyme
QA Contact: virgil.dicu
Blocks: 783874

Updated

5 years ago
Depends on: 788418
Depends on: 831095
Depends on: 841547
Blocks: 1089880
You need to log in before you can comment on or make changes to this bug.