Last Comment Bug 719552 - Scheme-less URLs references in the style sheet are lost when editing
: Scheme-less URLs references in the style sheet are lost when editing
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools: Style Editor (show other bugs)
: unspecified
: All All
: P1 normal (vote)
: Firefox 16
Assigned To: Cedric Vivier [:cedricv]
:
Mentors:
: 760994 (view as bug list)
Depends on: 727834
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-19 12:38 PST by Kevin Dangoor
Modified: 2012-06-12 04:51 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
patch v1 - use the new ParseStyleSheet API (6.46 KB, patch)
2012-02-23 04:15 PST, Cedric Vivier [:cedricv]
rcampbell: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Kevin Dangoor 2012-01-19 12:38:46 PST
If you change a background (on the HTML element, at least) with an image and then change it back, the background image does not come back.

STR:

1. go to http://hacks.mozilla.org/
2. edit the html {} rule that sets the background image (say remove the "u" in "url)
3. put the u back in

You'll see that the background image is gone, even though the rule is back to where it started.
Comment 1 Paul Rouget [:paul] 2012-01-26 04:48:24 PST
This happens only for non-absolute URLs. We are moving the style in an inline stylesheet. So some URL can become invalid.
Comment 2 Paul Rouget [:paul] 2012-01-27 03:18:06 PST
And I think this is also the reason why some font-faces stop working.

I would P1 that. It breaks quite a lot of websites and it makes the StyleEditor experience a bit "unpredictable".
Comment 3 Rob Campbell [:rc] (:robcee) 2012-01-27 06:04:40 PST
yeah, definitely bumping priority if this is causing breakage.
Comment 4 Rob Campbell [:rc] (:robcee) 2012-01-27 06:33:54 PST
ps, still open to recommendations on that method name!
Comment 5 Paul Rouget [:paul] 2012-01-27 06:38:33 PST
(In reply to Rob Campbell [:rc] (robcee) from comment #4)
> ps, still open to recommendations on that method name!

I think you meant to comment there: bug 705707
Comment 6 Rob Campbell [:rc] (:robcee) 2012-01-27 06:40:03 PST
uh disregard comment 4.
Comment 7 Cedric Vivier [:cedricv] 2012-02-23 04:15:32 PST
Created attachment 599948 [details] [diff] [review]
patch v1 - use the new ParseStyleSheet API
Comment 8 Rob Campbell [:rc] (:robcee) 2012-02-23 10:46:35 PST
Comment on attachment 599948 [details] [diff] [review]
patch v1 - use the new ParseStyleSheet API

nice.
Comment 9 Rob Campbell [:rc] (:robcee) 2012-02-23 10:49:17 PST
you might want to run this through try server before landing, but I leave that up to you. Not sure if your try run from the blocking bug includes this patch or not.
Comment 10 Cedric Vivier [:cedricv] 2012-02-23 10:51:13 PST
(In reply to Rob Campbell [:rc] (robcee) from comment #9)
> you might want to run this through try server before landing, but I leave
> that up to you. Not sure if your try run from the blocking bug includes this
> patch or not.

It does and it's all green yay :)
Comment 11 Paul Rouget [:paul] 2012-03-05 22:39:55 PST
Is this ready to land?
Comment 12 Cedric Vivier [:cedricv] 2012-03-06 02:48:18 PST
(In reply to Paul Rouget [:paul] from comment #11)
> Is this ready to land?

Nope, it has a dependency on bug 727834.
Comment 13 Paul Rouget [:paul] 2012-03-13 02:54:37 PDT
What are the chances to fix bug 727834 soon (before Firefox 14 cut)? If you think it will take time, can we get a temporary solution for this bug?
Comment 14 Paul Rouget [:paul] 2012-06-03 08:19:27 PDT
*** Bug 760994 has been marked as a duplicate of this bug. ***
Comment 15 Paul Rouget [:paul] 2012-06-06 08:50:09 PDT
Now that bug 727834 is fixed, can we land this patch?
Comment 16 Paul Rouget [:paul] 2012-06-06 08:51:51 PDT
And could we get that in 15?
Comment 17 Cedric Vivier [:cedricv] 2012-06-07 06:32:39 PDT
Yes and this is indeed a good candidate for aurora-approval as this is an important bug fix which consists of a small (more lines removed than added in actual code) and low-risk change on the JavaScript side.
Comment 19 Paul Rouget [:paul] 2012-06-07 09:08:25 PDT
Comment on attachment 599948 [details] [diff] [review]
patch v1 - use the new ParseStyleSheet API

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression (never worked correctly)
User impact if declined: the style editor fails often
Testing completed (on m-c, etc.): several try-runs, locally tested
Risk to taking this patch (and alternatives if risky): quite short, not too risky
String or UUID changes made by this patch: none
Comment 20 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2012-06-09 08:36:43 PDT
https://hg.mozilla.org/mozilla-central/rev/45d4ea8d17aa
Comment 21 Alex Keybl [:akeybl] 2012-06-11 13:37:01 PDT
Comment on attachment 599948 [details] [diff] [review]
patch v1 - use the new ParseStyleSheet API

[Triage Comment]
Early enough in the cycle to take a fix for Aurora 15, even though this has non-zero risk.

Note You need to log in before you can comment on or make changes to this bug.