1226543 - Openning the inspector's filter tooltip (to edit a SVG url CSS filter) breaks "data:" URLs

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[arni2033]

Attachment: testcase 1 - Editing a SVG url CSS filter with the inspector's filter tooltip still changes the URL.htm

>>>   My Info:   Win7_64, Nightly 45, 32bit, ID 20151119030404
STR:
1. Open attached "testcase 1"
2. Open devtools -> Inspector, inspect <div>, open "Rules" tab in sidebar
3. Click the circle between "filter" and "url"

Result:       The value changed and the rule became invalid
> before clicking   url(data:image/svg+xml;utf8,<svg\ xmlns=\"http://www.w3.org/2000/svg\"><filter\ id=\"blur\"><feGaussianBlur\ stdDeviation=\"3\"/></filter></svg>#blur);
> after  clicking   url(data:image/svg+xml\;utf8,<svg\ xmlns=)"http://www.w3.org/2000/svg"><filter id="blur"><feGaussianBlur stdDeviation="3"/></filter></svg>#blur);

Expectations: Url should not change

Leaving unconfirmed because this is an edge case compared to 1221156

Comment 1

[YF (Yang)]

Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20151211030207 CSet: 754b4805a65cab4f3aca99899227acc44ba4fb20

Comment 2

[Tom Tromey :tromey]

The regexp in CSSFilterEditorWidget.getValueAt is wrong.
It doesn't properly quote all the not-valid-in-url characters.

Comment 3

[Tom Tromey :tromey]

Attachment: fix URL quoting in CSSFilterEditorWidget.getValueAt

Comment 4

[Tom Tromey :tromey]

Comment on attachment 8698542 [details] [diff] [review]
fix URL quoting in CSSFilterEditorWidget.getValueAt

The earlier regexp was not correct.  Instead it needs to follow the CSS
spec more closely.

I omitted non-printable characters from the regexp; but I could add those
easily if desired.

I've updated the test to also verify that the "weird" URLs we're testing
here do in fact lex as URL tokens.

Comment 5

[Patrick Brosset <:pbro>]

Comment on attachment 8698542 [details] [diff] [review]
fix URL quoting in CSSFilterEditorWidget.getValueAt

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

::: devtools/client/shared/test/browser_filter-editor-01.js
@@ +13,5 @@
> +      Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
> +
> +// Verify that the given string consists of a valid CSS URL token.
> +// Return true on success, false on error.
> +function verifyURL(string) {

This function tests the lexer, right? Not really the filter editor tooltip.
It seems out of place. Do we have xpcshell tests for the lexer? Or other kinds of tests? Isn't this tested somewhere else already?

Comment 6

[Tom Tromey :tromey]

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #5)

> This function tests the lexer, right? Not really the filter editor tooltip.
> It seems out of place. Do we have xpcshell tests for the lexer? Or other
> kinds of tests? Isn't this tested somewhere else already?

The idea here is that in addition to testing that the filter widget does the
right thing, we want to know that the CSS we're passing to the filter widget
is actually correct.  Putting the test somewhere else would mean that this
consistency check would be non-obvious.

Comment 7

[Tom Tromey :tromey]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=887cbbbac20e

Comment 8

[Pulsebot]

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

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c11fb05612bf

Comment 10

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> VERIFIED

Comments:
Test successful


Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel	beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS				Windows 7 SP1 x86_64

Expected Results: 
No change in URL detected after clicking.

Actual Results: 
As expected