Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

script: Implement the HTMLTrackElement API #22563

Merged
merged 1 commit into from Jan 8, 2019

Conversation

@dlrobertson
Copy link
Contributor

@dlrobertson dlrobertson commented Dec 26, 2018

Implement the basics of the HTMLTrackElement and update the wpt tests.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

Fixes: #22313


This change is Reviewable

@highfive
Copy link

@highfive highfive commented Dec 26, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webidls/HTMLTrackElement.webidl, components/script/dom/htmltrackelement.rs
  • @KiChjang: components/script/dom/webidls/HTMLTrackElement.webidl, components/script/dom/htmltrackelement.rs
@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Dec 26, 2018

Some of the async tests for activeCue etc were falsely passing. Updated the test expectations.

@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Dec 26, 2018

r? @ferjm

@highfive highfive assigned ferjm and unassigned asajeffrey Dec 26, 2018
@dlrobertson dlrobertson force-pushed the dlrobertson:htmltextelement branch from c32d063 to 235ea4d Dec 26, 2018
@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Dec 26, 2018

Sorry, thought I ran test-tidy. Updated.

@ferjm ferjm added this to In progress in Media playback Dec 27, 2018
@ferjm
ferjm approved these changes Dec 28, 2018
Copy link
Member

@ferjm ferjm left a comment

Excellent!

// https://html.spec.whatwg.org/multipage/#dom-track-kind
fn Kind(&self) -> DOMString {
let element = self.upcast::<Element>();
// The the value of "kind" and transform all uppercase

This comment has been minimized.

@ferjm

ferjm Dec 28, 2018
Member

typo: duplicated the. I guess you meant to write "Get the value..."

This comment has been minimized.

@dlrobertson

dlrobertson Dec 28, 2018
Author Contributor

Good catch

) -> HTMLTrackElement {
HTMLTrackElement {
htmlelement: HTMLElement::new_inherited(local_name, prefix, document),
ready_state: HTMLTrackElementConstants::NONE,

This comment has been minimized.

@dlrobertson

dlrobertson Dec 28, 2018
Author Contributor

I can definitely add that. In this PR the enum would be mostly unused, since None is the only variant that is currently used.

// https://html.spec.whatwg.org/multipage/#dom-track-src
make_url_getter!(Src, "src");
// https://html.spec.whatwg.org/multipage/#dom-track-src
make_url_setter!(SetSrc, "src");

This comment has been minimized.

@ferjm

ferjm Dec 28, 2018
Member

I believe we depend on implementing and using Element::set_url_attribute within make_url_setter to be fully spec compliant here. Could you file a follow up bug to implement this, please?

This comment has been minimized.

@dlrobertson

dlrobertson Dec 28, 2018
Author Contributor

Yeah, for now make_url_setter is the same as make_setter.

@ferjm
Copy link
Member

@ferjm ferjm commented Dec 28, 2018

@bors-servo try=wpt

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Dec 28, 2018

Trying commit 235ea4d with merge 27fc007...

bors-servo added a commit that referenced this pull request Dec 28, 2018
script: Implement the HTMLTrackElement API

Implement the basics of the HTMLTrackElement and update the wpt tests.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

Fixes: #22313

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22563)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Dec 28, 2018

💔 Test failed - linux-rel-css

@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Jan 1, 2019

@ferjm Updated

@ferjm
Copy link
Member

@ferjm ferjm commented Jan 7, 2019

It seems that this makes some tests in /html/infrastructure/urls/dynamic-changes-to-base-urls pass.

{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "The 'src' attribute of the 'track' element", 
    "test": "/html/infrastructure/urls/dynamic-changes-to-base-urls/dynamic-urls.sub.html", 
    "line": 94427, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "The 'src' attribute of the 'track' element", 
    "test": "/html/infrastructure/urls/dynamic-changes-to-base-urls/historical.sub.xhtml", 
    "line": 95674, 
    "action": "test_result", 
    "expected": "FAIL"
}
@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Jan 7, 2019

These timeouts also might be related to this change. The tests include TextTracks and I don't think the previous PASS was spurious.

{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "<video autoplay> with <track src=\"invalid://url\" default=\"\"> child", 
    "test": "/html/semantics/embedded-content/media-elements/autoplay-with-broken-track.html", 
    "line": 123088, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "<video autoplay> with <track src=\"404\" default=\"\"> child", 
    "test": "/html/semantics/embedded-content/media-elements/autoplay-with-broken-track.html", 
    "line": 123089, 
    "action": "test_result", 
    "expected": "PASS"
}
{
    "status": "TIMEOUT", 
    "group": "default", 
    "message": "Test timed out", 
    "stack": null, 
    "subtest": "<video autoplay> with <track src=\"\" default=\"\"> child", 
    "test": "/html/semantics/embedded-content/media-elements/autoplay-with-broken-track.html", 
    "line": 123090, 
    "action": "test_result", 
    "expected": "PASS"
}
@dlrobertson dlrobertson force-pushed the dlrobertson:htmltextelement branch from b9f06d6 to ab787bf Jan 7, 2019
@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Jan 7, 2019

@ferjm Updated

@ferjm
Copy link
Member

@ferjm ferjm commented Jan 8, 2019

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

📌 Commit ab787bf has been approved by ferjm

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

Testing commit ab787bf with merge 26ece56...

bors-servo added a commit that referenced this pull request Jan 8, 2019
script: Implement the HTMLTrackElement API

Implement the basics of the HTMLTrackElement and update the wpt tests.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

Fixes: #22313

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22563)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

💔 Test failed - linux-rel-wpt

@ferjm
Copy link
Member

@ferjm ferjm commented Jan 8, 2019

We need to update the expectations for html/dom/interfaces.https.html as well

@CYBAI
Copy link
Member

@CYBAI CYBAI commented Jan 8, 2019

Looks like there're more PASSed test cases now :D

{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack must be primary interface of document.createElement(\"track\").track", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72018, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "Stringification of document.createElement(\"track\").track", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72019, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"kind\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72020, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"label\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72021, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"language\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72022, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"id\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72023, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"mode\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72025, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"cues\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72026, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"addCue(TextTrackCue)\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72028, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: calling addCue(TextTrackCue) on document.createElement(\"track\").track with too few arguments must throw TypeError", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72029, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"removeCue(TextTrackCue)\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72030, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: calling removeCue(TextTrackCue) on document.createElement(\"track\").track with too few arguments must throw TypeError", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72031, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "TextTrack interface: document.createElement(\"track\").track must inherit property \"oncuechange\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?exclude=(Document|Window|HTML.*)", 
    "line": 72032, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute kind", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80526, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute src", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80527, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute srclang", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80528, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute label", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80529, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute default", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80530, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant NONE on interface object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80531, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant NONE on interface prototype object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80532, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant LOADING on interface object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80533, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant LOADING on interface prototype object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80534, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant LOADED on interface object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80535, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant LOADED on interface prototype object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80536, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant ERROR on interface object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80537, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: constant ERROR on interface prototype object", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80538, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute readyState", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80539, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: attribute track", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80540, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"kind\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80543, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"src\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80544, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"srclang\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80545, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"label\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80546, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"default\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80547, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"NONE\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80548, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"LOADING\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80549, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"LOADED\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80550, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"ERROR\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80551, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"readyState\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80552, 
    "action": "test_result", 
    "expected": "FAIL"
}
{
    "status": "PASS", 
    "group": "default", 
    "message": null, 
    "stack": null, 
    "subtest": "HTMLTrackElement interface: document.createElement(\"track\") must inherit property \"track\" with the proper type", 
    "test": "/html/dom/interfaces.https.html?include=HTML.*", 
    "line": 80553, 
    "action": "test_result", 
    "expected": "FAIL"
}
Implement the basics of the HTMLTrackElement and update the wpt tests.
@dlrobertson dlrobertson force-pushed the dlrobertson:htmltextelement branch from ab787bf to c480207 Jan 8, 2019
@dlrobertson
Copy link
Contributor Author

@dlrobertson dlrobertson commented Jan 8, 2019

@ferjm @CYBAI thanks, updated the html/dom/interfaces.https.html test expectations.

@ferjm
Copy link
Member

@ferjm ferjm commented Jan 8, 2019

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

📌 Commit c480207 has been approved by ferjm

@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

Testing commit c480207 with merge 7c8de38...

bors-servo added a commit that referenced this pull request Jan 8, 2019
script: Implement the HTMLTrackElement API

Implement the basics of the HTMLTrackElement and update the wpt tests.

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

Fixes: #22313

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22563)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

@bors-servo bors-servo commented Jan 8, 2019

@bors-servo bors-servo merged commit c480207 into servo:master Jan 8, 2019
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@bors-servo
homu Test successful
Details
Media playback automation moved this from In progress to Done Jan 8, 2019
@dlrobertson dlrobertson deleted the dlrobertson:htmltextelement branch Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants