Closed
Bug 516665
Opened 15 years ago
Closed 15 years ago
distorted images with moz-icon://*?size=dialog
Categories
(Core :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta1+ |
People
(Reporter: bugzilla.i.sekler, Assigned: ginnchen+exoracle)
References
()
Details
(Keywords: regression, Whiteboard: [decodeondraw])
Attachments
(4 files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
bholley
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090915 Firefox/3.7a1pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090915 Firefox/3.7a1pre
The display of images like moz-icon://stock/gtk-dialog-warning?size=dialog shows pixel noise in the lower part of the image (screenshots will follow). The same image over file:// displays fine.
This happend after <http://hg.mozilla.org/mozilla-central/rev/ff3496b1f6c7> and at or before <http://hg.mozilla.org/mozilla-central/rev/3655f5de6392>
Maybe something in connection with the new decode-on-draw?
Reproducible: Always
Steps to Reproduce:
1. Load moz-icon://stock/gtk-dialog-warning?size=dialog
Actual Results:
The lower part of the image is broken (contains a blank stripe and pixel noise below).
The images that show the bug belong to the "Human" GNOME theme of Ubuntu 9.04. Can't tell whether they are required to be able to reproduce the bug yet.
Reporter | ||
Comment 1•15 years ago
|
||
Reporter | ||
Comment 2•15 years ago
|
||
Reporter | ||
Updated•15 years ago
|
Keywords: regression
Version: unspecified → Trunk
Comment 4•15 years ago
|
||
what's the equivalent of the gtk-dialog-warning string for other platforms?
Joe - does the icon url mean that this somehow gets decoded with the
nsIconDecoder? If so, then it makes sense that this would be broken, since I
rewrote the icon decoder. Otherwise, I'm not sure why loading it with a
different URL makes a difference. Normally it might have to do with our use of
the false/false configuration for chrome images, but _everything_ is
false/false right now.
Updated•15 years ago
|
Whiteboard: [decodeondraw]
Comment 5•15 years ago
|
||
yeah so at first glance it looks like this goes through the nsIconDecoder, meaning it's almost definitely a decode-on-draw regression. On the list.
Updated•15 years ago
|
Severity: normal → major
blocking2.0: --- → ?
Comment 6•15 years ago
|
||
Since I just tested, this regressed between Linux x86-64 nightlies 2009-09-13-03-mozilla-central and 2009-09-14-03-mozilla-central, which is the range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf0fdec8f43b&tochange=7f0988f042e9
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 7•15 years ago
|
||
Reverting the changes to modules/libpr0n/src/imgRequest.cpp from the Bug 516311 http://hg.mozilla.org/mozilla-central/rev/f34cc41267d8 fixes the issue.
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090920 Firefox/3.7a1pre
SourceStamp=9061f43b86bd+
Comment 8•15 years ago
|
||
(In reply to comment #7)
> Reverting the changes to modules/libpr0n/src/imgRequest.cpp from the Bug 516311
> http://hg.mozilla.org/mozilla-central/rev/f34cc41267d8 fixes the issue.
>
> Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090920
> Firefox/3.7a1pre
>
> SourceStamp=9061f43b86bd+
I have exactly the same results. It seems to me that code checked in to fix a performance only regression on one platform, that causes a real non-performance issue on another platform needs to be backed out immediately.
Blocks: 516311
Comment 9•15 years ago
|
||
(In reply to comment #8)
> (In reply to comment #7)
> I have exactly the same results. It seems to me that code checked in to fix a
> performance only regression on one platform, that causes a real non-performance
> issue on another platform needs to be backed out immediately.
I don't see why this is necessarily the case.
Regardless, that isn't actually the issue here. This bug was most certainly caused by my rewrite of the icon decoder in bug 435296. The checkin mentioned here was just toggling a global variable that switched a code patch to a more known one, but apparently one that didn't quite work properly with the icon decoder rewrite. Joe's going to look into this, it will almost certainly be a simple fix once we figure out what's going on, and we of course wouldn't ship a production browser with this bug unresolved.
Comment 10•15 years ago
|
||
(In reply to comment #9)
> Regardless, that isn't actually the issue here. This bug was most certainly
> caused by my rewrite of the icon decoder in bug 435296. The checkin mentioned
> here was just toggling a global variable that switched a code patch to a more
> known one, but apparently one that didn't quite work properly with the icon
> decoder rewrite. Joe's going to look into this, it will almost certainly be a
> simple fix once we figure out what's going on, and we of course wouldn't ship a
> production browser with this bug unresolved.
Yes, Sorry, I did not realize that the patch in questions was merely intended to turn off the decode on draw optimization,
So, they images in question draw correctly either without the decode-on-draw code int he build, or with it in the build and enabled, but not in the case where it is in the build but disabled. This is obviously something that needs to be corrected. I should have actually looked at the patch rather than just doing an hg bisect to identify it.
Comment 13•15 years ago
|
||
From all the dupe reports we've been getting, it looks like this is probably linux specific.
Updated•15 years ago
|
Assignee | ||
Comment 15•15 years ago
|
||
I think the typo here is the problem.
Comment 17•15 years ago
|
||
Comment on attachment 406996 [details] [diff] [review]
patch
r=bholley. Does this fix the issue, if so, this can land right away.
Ginn - do you have commit access? If not joe or I can push this today (probably joe since I'll be out and about until 7pm).
Attachment #406996 -
Flags: review?(bobbyholley) → review+
Comment 18•15 years ago
|
||
FWIW -- this patch doesn't apply cleanly to mozilla-central -- it looks like it was generated from a checkout that lacked Bug 512269's changes. But luckily, this patch is just a 3-character change, so it's easy to update to tip. :)
> r=bholley. Does this fix the issue
I've just tested (an un-bitrotted version of) Ginn's patch on a Linux mozilla-central debug build, and I can confirm that it fixes this bug for moz-icon://stock/gtk-dialog-warning?size=dialog
> Ginn - do you have commit access?
Yeah, he does. (see bug 514632 comment 6)
Comment 19•15 years ago
|
||
Same patch. This one applies cleanly to current mozilla-central tip.;
Assignee | ||
Comment 20•15 years ago
|
||
Thanks, I forgot to switch to HEAD after tracing the regression.
It was a 2-character change actually. :)
Pushed:
http://hg.mozilla.org/mozilla-central/rev/09905e5b1f98
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
blocking2.0: ? → beta1
You need to log in
before you can comment on or make changes to this bug.
Description
•