Update src/href attributes to be a USVString #22456

dlrobertson · 2018-12-14T04:10:34Z

The following IDLs have the src/href attributes typed as a DOMString
while in the spec the attribute has been updated to be a USVString:

Fixes: #22454

This change is

highfive · 2018-12-14T04:10:38Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/htmliframeelement.rs, components/script/dom/htmllinkelement.rs, components/script/dom/webidls/HTMLScriptElement.webidl, components/script/dom/macros.rs, components/script/dom/webidls/HTMLInputElement.webidl and 10 more
@KiChjang: components/script/dom/htmliframeelement.rs, components/script/dom/htmllinkelement.rs, components/script/dom/webidls/HTMLScriptElement.webidl, components/script/dom/macros.rs, components/script/dom/webidls/HTMLInputElement.webidl and 10 more

highfive · 2018-12-14T04:10:40Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

ferjm

LGTM. There are a couple of DOMStrings that should be USVStrings left though. Thanks!

ferjm · 2018-12-14T07:09:41Z

components/script/dom/webidls/HTMLMediaElement.webidl

@@ -13,7 +13,7 @@ interface HTMLMediaElement : HTMLElement {
  readonly attribute MediaError? error;

  // network state
-  [CEReactions] attribute DOMString src;
+  [CEReactions] attribute USVString src;
  attribute MediaProvider? srcObject;
  readonly attribute DOMString currentSrc;


According to the spec, currentSrc should also be an USVString

ferjm · 2018-12-14T07:09:41Z

components/script/dom/webidls/HTMLImageElement.webidl

@@ -8,7 +8,7 @@ interface HTMLImageElement : HTMLElement {
  [CEReactions]
           attribute DOMString alt;
  [CEReactions]
-           attribute DOMString src;
+           attribute USVString src;
  [CEReactions]
           attribute DOMString srcset;


srcset and currentSrc should also be USVStrings

SimonSapin · 2018-12-14T17:17:40Z

Testing mozilla/treeherder#4371

@bors-servo try=linux

bors-servo · 2018-12-14T17:17:46Z

⌛ Trying commit 86f715a with merge d443b2a...


        Auto merge of #22456 - dlrobertson:fix_src, r=<try>

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

bors-servo · 2018-12-14T17:27:32Z

💔 Test failed - status-taskcluster

SimonSapin · 2018-12-14T17:28:10Z

@bors-servo try=linux retry

SimonSapin · 2018-12-14T17:28:28Z

@bors-servo try-

SimonSapin · 2018-12-14T17:28:33Z

@bors-servo try=linux

bors-servo · 2018-12-14T18:31:19Z

⌛ Trying commit 86f715a with merge 8ca7be9...


        Auto merge of #22456 - dlrobertson:fix_src, r=<try>

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

dlrobertson · 2018-12-14T22:44:03Z

@ferjm updated

ferjm

Looks good. Thanks!

ferjm · 2018-12-17T13:47:25Z

components/script/dom/bindings/str.rs

+    }
+}
+
+impl<'a> From<&'a str> for USVString {


What do we need this for? I can't see us using it at the moment.

ferjm · 2018-12-17T13:47:25Z

components/script/dom/bindings/str.rs

+    }
+}
+
+impl<'a> From<Cow<'a, str>> for USVString {


We do not seem to need this either.


        Update src/href attributes to be a USVString

The following IDLs have the src/href attributes typed as a DOMString while in the spec the attribute has been updated to be a USVString: - HTMLIFrameElement - HTMLImageElement - HTMLInputElement - HTMLLinkElement - HTMLMediaElement - HTMLScriptElement

dlrobertson · 2018-12-17T15:37:29Z

👍 Removed them. They were implemented for DOMString, so I thought they might be useful down the road, but we can add them later if we need them.

ferjm · 2018-12-17T15:41:42Z

Thanks!

@bors-servo r+

bors-servo · 2018-12-17T15:41:44Z

📌 Commit c46508e has been approved by ferjm


        Auto merge of #22456 - dlrobertson:fix_src, r=ferjm

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

bors-servo · 2018-12-22T12:38:16Z

💔 Test failed - mac-rel-css1

jdm · 2018-12-22T14:33:30Z

@bors-servo retry

bors-servo · 2018-12-22T20:41:48Z

⌛ Testing commit c46508e with merge 1751958...


        Auto merge of #22456 - dlrobertson:fix_src, r=ferjm

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

bors-servo · 2018-12-22T21:19:43Z

💔 Test failed - mac-rel-wpt3

jdm · 2018-12-22T23:52:57Z

@bors-servo retry

bors-servo · 2018-12-23T00:57:14Z

⌛ Testing commit c46508e with merge 13fd9b0...


        Auto merge of #22456 - dlrobertson:fix_src, r=ferjm

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

bors-servo · 2018-12-23T01:48:03Z

💔 Test failed - mac-rel-wpt4

jdm · 2018-12-23T04:27:25Z

@bors-servo retry

#22537


        Auto merge of #22456 - dlrobertson:fix_src, r=ferjm

Update src/href attributes to be a USVString The following IDLs have the src/href attributes typed as a `DOMString` while in the spec the attribute has been updated to be a `USVString`: - [HTMLIFrameElement] - [HTMLImageElement] - [HTMLInputElement] - [HTMLLinkElement] - [HTMLMediaElement] - [HTMLScriptElement] Fixes: #22454 [HTMLIFrameElement]: https://html.spec.whatwg.org/multipage/#htmliframeelement [HTMLImageElement]: https://html.spec.whatwg.org/multipage/#htmlimageelement [HTMLInputElement]: https://html.spec.whatwg.org/multipage/#htmlinputelement [HTMLLinkElement]: https://html.spec.whatwg.org/multipage/#htmllinkelement [HTMLMediaElement]: https://html.spec.whatwg.org/multipage/#htmlmediaelement [HTMLScriptElement]: https://html.spec.whatwg.org/multipage/#htmlscriptelement  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22456)

bors-servo · 2018-12-23T04:50:43Z

⌛ Testing commit c46508e with merge 2757795...

bors-servo · 2018-12-23T06:00:07Z

☀️ Test successful - android-mac, arm32, arm64, linux-rel-css, linux-rel-wpt, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, magicleap, status-taskcluster
Approved by: ferjm
Pushing 2757795 to master...

highfive assigned avadacatavra Dec 14, 2018

highfive added the S-awaiting-review label Dec 14, 2018

ferjm requested changes Dec 14, 2018

View changes

ferjm added S-needs-code-changes and removed S-awaiting-review labels Dec 14, 2018

ferjm assigned ferjm and unassigned avadacatavra Dec 14, 2018

highfive added the S-tests-failed label Dec 14, 2018

dlrobertson force-pushed the dlrobertson:fix_src branch from 86f715a to aa8bcab Dec 14, 2018

highfive added S-awaiting-review and removed S-tests-failed S-needs-code-changes labels Dec 14, 2018

ferjm approved these changes Dec 17, 2018

View changes

dlrobertson force-pushed the dlrobertson:fix_src branch from aa8bcab to c46508e Dec 17, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Dec 22, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Dec 22, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Dec 22, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Dec 22, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Dec 23, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Dec 23, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Dec 23, 2018

bors-servo merged commit c46508e into servo:master Dec 23, 2018
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Dec 23, 2018

servo / servo

Update src/href attributes to be a USVString #22456

Update src/href attributes to be a USVString #22456

dlrobertson commented Dec 14, 2018 •

edited by SimonSapin

highfive commented Dec 14, 2018

highfive commented Dec 14, 2018

ferjm left a comment

This comment has been minimized.

This comment has been minimized.

SimonSapin commented Dec 14, 2018

bors-servo commented Dec 14, 2018

bors-servo commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

bors-servo commented Dec 14, 2018

dlrobertson commented Dec 14, 2018

ferjm left a comment

This comment has been minimized.

This comment has been minimized.

dlrobertson commented Dec 17, 2018

ferjm commented Dec 17, 2018

bors-servo commented Dec 17, 2018

bors-servo commented Dec 22, 2018

jdm commented Dec 22, 2018

bors-servo commented Dec 22, 2018

bors-servo commented Dec 22, 2018

jdm commented Dec 22, 2018

bors-servo commented Dec 23, 2018

bors-servo commented Dec 23, 2018

jdm commented Dec 23, 2018

bors-servo commented Dec 23, 2018

bors-servo commented Dec 23, 2018

servo / servo

Join GitHub today

Update src/href attributes to be a USVString #22456

Update src/href attributes to be a USVString #22456

Conversation

dlrobertson commented Dec 14, 2018 • edited by SimonSapin

highfive commented Dec 14, 2018

highfive commented Dec 14, 2018

ferjm left a comment

This comment has been minimized.

ferjm Dec 14, 2018

This comment has been minimized.

ferjm Dec 14, 2018

SimonSapin commented Dec 14, 2018

bors-servo commented Dec 14, 2018

bors-servo commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

SimonSapin commented Dec 14, 2018

bors-servo commented Dec 14, 2018

dlrobertson commented Dec 14, 2018

ferjm left a comment

This comment has been minimized.

ferjm Dec 17, 2018

This comment has been minimized.

ferjm Dec 17, 2018

dlrobertson commented Dec 17, 2018

ferjm commented Dec 17, 2018

bors-servo commented Dec 17, 2018

bors-servo commented Dec 22, 2018

jdm commented Dec 22, 2018

bors-servo commented Dec 22, 2018

bors-servo commented Dec 22, 2018

jdm commented Dec 22, 2018

bors-servo commented Dec 23, 2018

bors-servo commented Dec 23, 2018

jdm commented Dec 23, 2018

bors-servo commented Dec 23, 2018

bors-servo commented Dec 23, 2018

dlrobertson commented Dec 14, 2018 •

edited by SimonSapin