Bug 214955

Summary: Remove unused JS and CSS files of media controls
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: MediaAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, aboxhall, apinheiro, benjamin, calvaris, cdumez, cfleizach, changseok, cmarcelo, darin, dmazzoni, eric.carlson, esprehn+autocc, ews-watchlist, glenn, graouts, graouts, gyuyoung.kim, hi, hta, jcraig, jdiggs, jenner, jer.noble, joepeck, kondapallykalyan, macpherson, menard, mkwst, pdr, peng.liu6, philipj, samuel_white, sergio, simon.fraser, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=182502
Attachments:
Description Flags
Patch
none
Patch
dbates: review+
[Patch] WIP
hi: commit-queue-
[Patch] WIP
hi: commit-queue-
[Patch] WIP
hi: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
eric.carlson: review+, ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch none

Description Peng Liu 2020-07-29 18:28:33 PDT 
Remove unused JS and CSS files of media controls
Comment 1 Peng Liu 2020-07-29 18:30:54 PDT 
Created attachment 405538 [details]
Patch
Comment 2 Peng Liu 2020-07-29 18:37:49 PDT 
Created attachment 405539 [details]
Patch
Comment 3 Daniel Bates 2020-08-03 17:33:31 PDT 
Comment on attachment 405539 [details]
Patch

Patch looks good. Looks like something is still using the files from the EWS failure.
Comment 4 Daniel Bates 2020-08-03 17:35:49 PDT 
Probably just need updated expectation...don't look too closely
Comment 5 Daniel Bates 2020-08-03 17:36:12 PDT 
I didn't look closely is what I meant ^^^
Comment 6 Radar WebKit Bug Importer 2020-08-05 18:29:22 PDT 
<rdar://problem/66604040>
Comment 7 Devin Rousso 2021-03-17 19:46:00 PDT 
Created attachment 423550 [details]
[Patch] WIP
Comment 8 Devin Rousso 2021-03-17 20:01:23 PDT 
Created attachment 423552 [details]
[Patch] WIP

rebase
Comment 9 Darin Adler 2021-03-17 21:40:15 PDT 
No ChangeLog yet.
Comment 10 Darin Adler 2021-03-17 21:40:40 PDT 
Should not set review? if not ready to review. The webkit-patch tool can trigger EWS without setting review?.
Comment 11 Devin Rousso 2021-03-18 00:28:49 PDT 
Created attachment 423563 [details]
[Patch] WIP

rebase (again)

also
 - combine the `RenderThemeMac` and `RenderThemeIOS` media controls functions into `RenderThemeCocoa` since they're now the same
 - fix the CSP issue by making sure that the `<img>` created by `iconService` is attached to the UA `shadowRoot` for `isInUserAgentShadowTree` before setting the `src`
 - fix the DerivedSources makefile to use all prerequisites for the modern media controls JS/CSS, not just ones edited since the last `make`
Comment 12 Devin Rousso 2021-03-18 13:59:42 PDT 
Created attachment 423649 [details]
Patch
Comment 13 Devin Rousso 2021-03-19 09:58:23 PDT 
Created attachment 423740 [details]
Patch
Comment 14 Eric Carlson 2021-03-19 10:44:13 PDT 
Comment on attachment 423740 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423740&action=review

> LayoutTests/ChangeLog:9
> +        Remove tests (and their expectations) that used `ModernMediaControlsEnabled`.

do you mean `ModernMediaControlsEnabled=false`?
Comment 15 Devin Rousso 2021-03-19 11:30:20 PDT 
Comment on attachment 423740 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423740&action=review

>> LayoutTests/ChangeLog:9
>> +        Remove tests (and their expectations) that used `ModernMediaControlsEnabled`.
> 
> do you mean `ModernMediaControlsEnabled=false`?

Yes, but there was only one test that had `ModernMediaControlsEnabled=true` :P

I'll be more explicit/clear.
Comment 16 Peng Liu 2021-03-19 13:31:32 PDT 
Comment on attachment 423740 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=423740&action=review

> Source/WebKit/ChangeLog:13
> +        Replace `ModernMediaControlsEnabled` setting with `ENABLE_MODERN_MEDIA_CONTROLS` build flag.

Nit. I guess you mean `MODERN_MEDIA_CONTROLS` flag here but there is the macro `ENABLE_MODERN_MEDIA_CONTROLS` in this patch. :-)
Comment 17 Devin Rousso 2021-03-19 13:37:36 PDT 
Created attachment 423768 [details]
Patch
Comment 18 Devin Rousso 2021-03-22 10:13:10 PDT 
Created attachment 423904 [details]
Patch
Comment 19 EWS 2021-03-22 15:17:58 PDT 
Committed r274810: <https://commits.webkit.org/r274810>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423904 [details].
Comment 20 Aakash Jain 2021-03-24 11:09:18 PDT 
(In reply to EWS from comment #19)
> Committed r274810: <https://commits.webkit.org/r274810>
This caused fast/parser/xml-colon-entity.html to start failing. EWS indicated this issue on previous version of patch on this bug in https://ews-build.webkit.org/#/builders/51/builds/9405

History: https://results.webkit.org/?suite=layout-tests&test=fast%2Fparser%2Fxml-colon-entity.html
Comment 21 Robert Jenner 2021-03-24 11:20:12 PDT 
Re-added the test expectations to Failure for "fast/parser/xml-colon-entity.html" that was removed with a patch from this bug. Test expectation re-added here:

https://trac.webkit.org/changeset/274951/webkit