719552 - Scheme-less URLs references in the style sheet are lost when editing

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect, P1)

Description

[Kevin Dangoor]

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]]

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]]

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)]

yeah, definitely bumping priority if this is causing breakage.

Comment 4

[Rob Campbell [:rc] (:robcee)]

ps, still open to recommendations on that method name!

Comment 5

[Paul Rouget [:paul]]

(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)]

uh disregard comment 4.

Comment 7

[Cedric Vivier [:cedricv]]

Attachment: patch v1 - use the new ParseStyleSheet API

Comment 8

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 599948 [details] [diff] [review]
patch v1 - use the new ParseStyleSheet API

nice.

Comment 9

[Rob Campbell [:rc] (:robcee)]

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]]

(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]]

Is this ready to land?

Comment 12

[Cedric Vivier [:cedricv]]

(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]]

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 15

[Paul Rouget [:paul]]

Now that bug 727834 is fixed, can we land this patch?

Comment 16

[Paul Rouget [:paul]]

And could we get that in 15?

Comment 17

[Cedric Vivier [:cedricv]]

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 18

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/45d4ea8d17aa

Comment 19

[Paul Rouget [:paul]]

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] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/45d4ea8d17aa

Comment 21

[Alex Keybl [:akeybl]]

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.

Comment 22

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/210f12d591a7