Closed Bug 1221156 Opened 4 years ago Closed 4 years ago

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

Categories

(DevTools :: Inspector, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: arni2033, Assigned: tromey)

References

Details

Attachments

(3 files, 2 obsolete files)

STR:   (Win7_64, Nightly 45, 32bit, ID 20151031030207, new profile)
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);
Before bug 1211796 (I guess) the STR resulted in the following invalid url (I still see that on Fx43):
> before 1211796    url(data:image/svg+xml;utf8,<svg%20xmlns='http://www.w3.org/2000/svg'><filter%20id='blur'><feGaussianBlur%20stdDeviation='3'/></filter></svg>#blur);

Expectations: Url should not change
Sorry for bugspam. Apparently, it wasn't 1211796 which changed the way of url breakage. It was 1217328
However, it's not "blocking", because I don't think it has made things way worse.
No longer blocks: 1211796
See Also: → 1217328
Summary: Editing a SVG url CSS filter with the inspector's filter tooltip still changes the URL → Openning the inspector's filter tooltip (to edit a SVG url CSS filter) breaks "data:" URLs
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
Attachment #8684954 - Flags: review?(pbrosset)
Comment on attachment 8684954 [details] [diff] [review]
make FilterWidget try to preserve URL quoting

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

::: devtools/client/shared/widgets/FilterWidget.js
@@ +824,3 @@
>      const {value, unit} = filter;
>  
>      return value + unit;

Took me a while to read this function, maybe I was just being dumb, but having this big block of logic in the middle threw me off. I didn't even see the returns at first.
I think this would be more readable (at least to me):

getValueAt: function(index) {
  let filter = this.filters[index];
  if (!filter) {
    return null;
  }

  // Just return the value + unit for non-url values.
  if (filter.name !== "url") {
    return filter.value + filter.unit;
  }

  // url values need to be quoted and escaped.
  if (filter.quote === "'") {
    return "'" + filter.value.replace(/\'/g, "\\'") + "'";
  } else if (filter.quote === "\"") {
    return "\"" + filter.value.replace(/\"/g, "\\\"") + "\"";
  }

  // Unquoted. This approach might change the original input --
  // for example the original might be over-quoted. But, this is
  // correct and probably good enough.
  return filter.value.replace(/[ \t(){};]/g, "\\$&");
}
Attachment #8684954 - Flags: review?(pbrosset) → review+
Updated per review.
Attachment #8685448 - Flags: review+
Rebase on top of patch for bug 1223076.
Attachment #8684954 - Attachment is obsolete: true
Attachment #8685448 - Attachment is obsolete: true
Attachment #8686160 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4efdfcadd82f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Depends on: 1226543
Has STR: --- → yes
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.