Closed Bug 1068230 Opened 10 years ago Closed 10 years ago

CSS background url does not work in FireFox 32 (button tag)

Categories

(Core :: Layout, defect)

32 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla35
blocking-b2g 2.0+
Tracking Status
firefox32 --- wontfix
firefox33 + fixed
firefox34 + fixed
firefox35 + fixed
b2g-v2.0 --- verified
b2g-v2.0M --- verified
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: wkaster, Assigned: mwu)

References

()

Details

(Keywords: regression)

Attachments

(8 files, 2 obsolete files)

Attached image FF32_bug.png
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140911151253

Steps to reproduce:

This clause works great on FF 31, but not in FF 32.  

background: url('../img/button_bg.gif') repeat-x scroll left top;



Actual results:

the button disapeared


Expected results:

button apear with the image as backgound
Please attach a short testcase, thanks
Flags: needinfo?(wkaster)
minimal bug test file (test.html)
Flags: needinfo?(wkaster)
Attached image background file
(In reply to Matthias Versen [:Matti] from comment #1)
> Please attach a short testcase, thanks

Ok, test file and backgound image are attached!
I can confirm the issue on Windows7 with Firefox32 and Firefox33beta but it works on Firefox trunk.

I will do a regression range search later at home if required
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Product: Firefox → Core
So this is a one-pixel-wide gif, right?  The button seems to disappear because the background is painted white and the page styles the text explicitly to be white...

Matti, if you have time to do a regression range, that would in fact be very helpful.
Flags: needinfo?(bugzilla)
My statement that it works on trunk is incorrect !

The rendering of the testcase in Firefox24ESR looks identical to IE11. 
After this regression range the testcase is rendered blank.

Last good revision: 4511d9ac1000
First bad revision: 79624417d247
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4511d9ac1000&tochange=79624417d247


and after this "fix range" the background is visible again but is rendered different compared to Firefox24ESR and IE11

Last good revision: dc352a7bf234 (2014-08-26)
First bad revision: daa84204a11a (2014-08-25)
Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dc352a7bf234&tochange=daa84204a11a

NOTE: bisect inbound builds failed for me :-(
Flags: needinfo?(bugzilla)
Well, at least that first regression range is pretty clear.

Michael, Seth, could you please take a look?

[Tracking Requested - why for this release]: Web compat regression with background painting.
Blocks: 994081
Flags: needinfo?(seth)
Flags: needinfo?(mwu)
Keywords: regression
I had issues finding the "fix range" with server failures, mozregression bugs and a slow connection so it took a while.

good: blank rendering
bad: background visible again but different

Last good revision: 3488976ecf0f
First bad revision: 3d51132a0099
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3488976ecf0f&tochange=3d51132a0099
I have no idea how to test this, but it seems correct based on my limited understanding of how the new drawing code is suppose to work. Will send off to try.
Attachment #8491085 - Flags: review?(seth)
Flags: needinfo?(mwu)
This patch applies on top of the patch from bug 1062886.
Comment on attachment 8491085 [details] [diff] [review]
Don't use the gfxContext transform in intermediate surface

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

This makes sense. Thanks Michael!

It'd be great if we could get a test for this one too. I'd expect a similar setup (repeating background created using a single-pixel image) to work using just a plain div.
Attachment #8491085 - Flags: review?(seth) → review+
Flags: needinfo?(seth)
Assignee: nobody → mwu
Well uh, I have a test for this, but it exposes another bug in the drawing code. If I can't figure it out, I'll just land with the test disabled.
Same thing, but with a disabled test. There's something weird going on with tiling - seems like tiled images are being drawn with less precision or just wrong somehow.
Attachment #8491085 - Attachment is obsolete: true
Turns out we do need a transform when generating intermediate surfaces for offset gifs..

Tests should pass now.
Attachment #8491761 - Attachment is obsolete: true
Attachment #8491807 - Flags: review?(seth)
Comment on attachment 8491807 [details] [diff] [review]
Don't use the gfxContext transform in intermediate surface, v3

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

Nice - this makes even more sense. =) Thanks for digging into this until the fix was 100% correct.

::: image/test/reftest/gif/tile-transform.html
@@ +3,5 @@
> +https://bugzilla.mozilla.org/show_bug.cgi?id=1068230
> +-->
> +<html>
> +<head>
> +  <title>Should not transform intermediate surface when tiling an image</title>

Should this description be revised now that we realize we do need a translation on the pattern? I don't have a great suggestion for how to word it, unfortunately.
Attachment #8491807 - Flags: review?(seth) → review+
https://hg.mozilla.org/mozilla-central/rev/9a4f85820092
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Flags: in-testsuite+
Michael, could you prepare uplift requests for aurora & beta? Thanks
Flags: needinfo?(mwu)
Comment on attachment 8491807 [details] [diff] [review]
Don't use the gfxContext transform in intermediate surface, v3

Approval Request Comment
[Feature/regressing bug #]: Regressed by bug 994081 / bug 1043560
[User impact if declined]: Valid gifs aren't rendered correctly in backgrounds
[Describe test coverage new/current, TBPL]: Test added for this scenario.
[Risks and why]: Low risk - fix only impacts this particular scenario.
[String/UUID change made/needed]: None.
Attachment #8491807 - Flags: approval-mozilla-beta?
Attachment #8491807 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mwu)
Attachment #8491807 - Flags: approval-mozilla-beta?
Attachment #8491807 - Flags: approval-mozilla-beta+
Attachment #8491807 - Flags: approval-mozilla-aurora?
Attachment #8491807 - Flags: approval-mozilla-aurora+
Bug 1062886 should be uplifted before this one.

BTW some quick notes on the test -

1. A div wasn't enough - I had to use a button to hit this particular path. Maybe related to clamping?
2. I also had to use a offset gif. They can be generated through ImageMagick with a crop.
https://hg.mozilla.org/releases/mozilla-aurora/rev/c059221c4637

Waiting on a branch patch in bug 1062886 before this can land on beta (assuming this doesn't need a branch patch as well). Also, please nominate this for b2g32 approval as well if you feel it's something that should be fixed for v2.0.
Here's a patch for beta.
Attachment #8491807 - Attachment is obsolete: true
Flags: needinfo?(mwu)
Attachment #8491807 - Attachment is obsolete: false
https://hg.mozilla.org/releases/mozilla-beta/rev/bca0649c9b79

This will need a bit more rebasing if you intend to get this uplifted to b2g32 as well.
[Blocking Requested - why for this release]:

Valid gifs aren't rendered correctly in backgrounds. See comment 23 for more details.
blocking-b2g: --- → 2.0?
Comment on attachment 8494657 [details] [diff] [review]
Don't use the gfxContext transform in intermediate surface, v3 (B2G32)

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Regressed by bug 994081
User impact if declined: Valid gifs aren't rendered correctly in backgrounds
Testing completed: Test added for this scenario.
Risk to taking this patch (and alternatives if risky): Low risk - fix only impacts this particular scenario.
String or UUID changes made by this patch: None.
Attachment #8494657 - Flags: approval-mozilla-b2g32?
blocking-b2g: 2.0? → 2.0+
Attachment #8494657 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
Unable to verify this issue. 

There is no available STR or resources for the QA team to test this issue.
QA Whiteboard: [QAnalyst-Triage?] [QAnalyst-verify-]
Flags: needinfo?(ktucker)
A test case was attached in comment 2. Comment 8 gives examples of correct and incorrect rendering.
and to make it easier I uploaded the testcase on my server and added the URl into the URL field of this report
Michael, 

The test case listed in Comment 2 is not there, the page is blank.  However, I used the web address listed in Comment 8's screenshot.  

The "Test" button I have seen at: mversen.de/mozilla/bug1068230/test.html renders correctly.  Does this mean the issue is fixed? If yes, I will post my environmental variables.  Thanks for your time!
Flags: needinfo?(mwu)
Duane: 
It's a good idea to reproduce a reported bug in an affected Firefox version before verifying that the issue got fixed in Firefox versions that are marked as fixed. 
You can see in the bug header at the top of this bug report that this bug was reported for "Version: 32 Branch" and that is Firefox32. You see the same version string in the "User Agent" field of comment#0

>The test case listed in Comment 2 is not there, the page is blank.
You have to download the html file from comment#2 and also the image file from comment#3 and then make sure that the referenced image file in the html file matches the filename and path of the image file. 
It's easier to just use http://mversen.de/mozilla/bug1068230/test.html . That is the same testcase as comment#2

I can confirm that the bug is fixed with a current trunk (Firefox36) build and Firefox33.0.2 Release on Windows7
Flags: needinfo?(mwu)
Issue is verified fixed in Flame 2.0, 2.1, 2.2  (Full Flash, nightly). 

Actual Results: The test case located at <http://mversen.de/mozilla/bug1068230/test.html> renders correctly (verifying issue based on Comment 8's attachment). 

Note: I determined this issue was affecting the Flame prior to 9/19 and is now fixed in the builds listed below. 

Device: Flame 2.0
BuildID: 20141104000201
Gaia: fe2167fa5314c7e71c143a590914cbf3771905a8
Gecko: 241e51806687
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 32.0 (2.0)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 2.1
BuildID: 20141104001202
Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5
Gecko: 388b03efe92d
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2
BuildID: 20141104040207
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 54d05732f29b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?] [QAnalyst-verify-] → [QAnalyst-Triage?]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Verify passed, this issue can't be repro on Woodduck 2.0.
Attached: Verify_Woodduck_CSS.mp4
Note:The test case located at <http://mversen.de/mozilla/bug1068230/test.html> renders correctly (Verifying issue with Comment 8's attachment)
Reproducing rate: 0/5

Woodduck build:
Gaia-Rev        3a98f1287fa7b604891220ba5d86982ae8f9971e
Gecko-Rev       03d3ab62d5b07b915434f2d1d68495ad5915ecd2
Build-ID        20141120103003
Version         32.0
Attached video Verify_Woodduck_CSS.mp4
You need to log in before you can comment on or make changes to this bug.