1323369 - Enable pause-remove-from-document-networkState.html

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

The Gecko codes should follow WhatWG HTML spec not HTML5 spec. In WhatWG HTML, no check of NETWORK_EMPTY anymore.

See:

When a media element is removed from a Document, the user agent must run the following steps:

Await a stable state, allowing the task that removed the media element from the Document to continue. The synchronous section consists of all the remaining steps of this algorithm. (Steps in the synchronous section are marked with ⌛.)

⌛ If the media element is in a document, abort these steps.

⌛ Run the internal pause steps for the media element.

Comment 1

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Bug 1323369 - Make HTMLMediaElement::UnbindFromTree to follow WhatWG HTML spec.

W3C HTML5 spec isn't very actively maintained now. WhatWG HTML spec:
When a media element is removed from a Document, the user agent must run the following steps:
Below is the related statement in WhatWG HTML spec:
Await a stable state, allowing the task that removed the media element from the Document to continue. The synchronous section consists of all the remaining steps of this algorithm. (Steps in the synchronous section are marked with ⌛.)
⌛ If the media element is in a document, abort these steps.
⌛ Run the internal pause steps for the media element.

Review commit: https://reviewboard.mozilla.org/r/98516/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98516/

Comment 2

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Attachment: Bug 1323369 - Enable pause-remove-from-document-networkState.html.

Review commit: https://reviewboard.mozilla.org/r/98518/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98518/

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Chia-hung Tai [:ctai :ctai_mozilla :cht] from comment #0)
> The Gecko codes should follow WhatWG HTML spec not HTML5 spec. In WhatWG
> HTML, no check of NETWORK_EMPTY anymore.

I am not sure which spec. the wpt follows.

See bug 1304134 comment 9 and bug 1304134 comment 10. Somehow the test is broken if we follow whatwg.

Comment 4

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Smaug,
You might also comment on the spec issue. Thanks.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Gecko should follow WhatWG HTML spec since that is the spec properly maintained. W3C HTML5 spec tends to be just a snapshot of HTML spec, meaning that it contains more bugs.

Comment 6

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Sounds very reasonable. JW and cpearce what do you think?

(In reply to Olli Pettay [:smaug] from comment #5)
> Gecko should follow WhatWG HTML spec since that is the spec properly
> maintained. W3C HTML5 spec tends to be just a snapshot of HTML spec, meaning
> that it contains more bugs.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8818467 [details]
Bug 1323369 - Make HTMLMediaElement::UnbindFromTree to follow WhatWG HTML spec.

https://reviewboard.mozilla.org/r/98516/#review99052

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8818468 [details]
Bug 1323369 - Enable pause-remove-from-document-networkState.html.

https://reviewboard.mozilla.org/r/98518/#review99054

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Let's see if any test will be broken.

Comment 10

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

If so, then you should file a bug to fix the test case.
Doesn't it, smaug, JW?
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #9)
> Let's see if any test will be broken.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Not sure what I'm supposed to comment here.
If our code follows the spec and there are failing tests, that in general means that tests need to be
fixed. If other browsers have different behavior, it would be nice to file bugs on their bug trackers.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6bf4393db489
Make HTMLMediaElement::UnbindFromTree to follow WhatWG HTML spec. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/8f5ee59daee2
Enable pause-remove-from-document-networkState.html. r=jwwang

Comment 13

[Carsten Book [:Tomcat]]

had to back this out for leaks like https://treeherder.mozilla.org/logviewer.html#?job_id=7949571&repo=autoland that started with this push

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/507ed4bc9275
Backed out changeset 8f5ee59daee2 
https://hg.mozilla.org/integration/autoland/rev/180cdc0296a3
Backed out changeset 6bf4393db489 for causing memory leaks on a CLOSED TREE

Comment 15

[Chris Pearce [:cpearce (Not reading bugmail)]]

I agree with Olli. Clearing need-info.

Comment 16

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Fixed. See try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1a70d43f9350fe19d2cd58a452437acc9dd5e90

(In reply to Carsten Book [:Tomcat] from comment #13)
> had to back this out for leaks like
> https://treeherder.mozilla.org/logviewer.html#?job_id=7949571&repo=autoland
> that started with this push

Comment 17

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Comment on attachment 8818467 [details]
Bug 1323369 - Make HTMLMediaElement::UnbindFromTree to follow WhatWG HTML spec.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98516/diff/1-2/

Comment 18

[Chia-hung Tai [:ctai :ctai_mozilla :cht]]

Comment on attachment 8818468 [details]
Bug 1323369 - Enable pause-remove-from-document-networkState.html.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/98518/diff/1-2/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15ed85da015c
Make HTMLMediaElement::UnbindFromTree to follow WhatWG HTML spec. r=jwwang
https://hg.mozilla.org/integration/autoland/rev/67777e24d05c
Enable pause-remove-from-document-networkState.html. r=jwwang

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15ed85da015c
https://hg.mozilla.org/mozilla-central/rev/67777e24d05c