RESOLVED FIXED 214955
Remove unused JS and CSS files of media controls 
https://bugs.webkit.org/show_bug.cgi?id=214955
Summary Remove unused JS and CSS files of media controls
Peng Liu
Reported 2020-07-29 18:28:33 PDT
Remove unused JS and CSS files of media controls
Attachments
Patch (346.55 KB, patch)
2020-07-29 18:30 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
Patch (211.65 KB, patch)
2020-07-29 18:37 PDT, Peng Liu 		dbates: review+
DetailsFormatted DiffDiff
[Patch] WIP (1.21 MB, patch)
2021-03-17 19:46 PDT, Devin Rousso 		hi: commit-queue-
DetailsFormatted DiffDiff
[Patch] WIP (1.21 MB, patch)
2021-03-17 20:01 PDT, Devin Rousso 		hi: commit-queue-
DetailsFormatted DiffDiff
[Patch] WIP (1.21 MB, patch)
2021-03-18 00:28 PDT, Devin Rousso 		hi: commit-queue-
DetailsFormatted DiffDiff
Patch (1.25 MB, patch)
2021-03-18 13:59 PDT, Devin Rousso 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (1.22 MB, patch)
2021-03-19 09:58 PDT, Devin Rousso 		eric.carlson: review+
ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (1.23 MB, patch)
2021-03-19 13:37 PDT, Devin Rousso 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (1.23 MB, patch)
2021-03-22 10:13 PDT, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.
Peng Liu
Comment 1 2020-07-29 18:30:54 PDT
Created attachment 405538 [details] Patch
Peng Liu
Comment 2 2020-07-29 18:37:49 PDT
Created attachment 405539 [details] Patch
Daniel Bates
Comment 3 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.
Daniel Bates
Comment 4 2020-08-03 17:35:49 PDT
Probably just need updated expectation...don't look too closely
Daniel Bates
Comment 5 2020-08-03 17:36:12 PDT
I didn't look closely is what I meant ^^^
Radar WebKit Bug Importer
Comment 6 2020-08-05 18:29:22 PDT
<rdar://problem/66604040>
Devin Rousso
Comment 7 2021-03-17 19:46:00 PDT
Created attachment 423550 [details] [Patch] WIP
Devin Rousso
Comment 8 2021-03-17 20:01:23 PDT
Created attachment 423552 [details] [Patch] WIP rebase
Darin Adler
Comment 9 2021-03-17 21:40:15 PDT
No ChangeLog yet.
Darin Adler
Comment 10 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?.
Devin Rousso
Comment 11 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`
Devin Rousso
Comment 12 2021-03-18 13:59:42 PDT
Created attachment 423649 [details] Patch
Devin Rousso
Comment 13 2021-03-19 09:58:23 PDT
Created attachment 423740 [details] Patch
Eric Carlson
Comment 14 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`?
Devin Rousso
Comment 15 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.
Peng Liu
Comment 16 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. :-)
Devin Rousso
Comment 17 2021-03-19 13:37:36 PDT
Created attachment 423768 [details] Patch
Devin Rousso
Comment 18 2021-03-22 10:13:10 PDT
Created attachment 423904 [details] Patch
EWS
Comment 19 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].
Aakash Jain
Comment 20 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
Robert Jenner
Comment 21 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
Note You need to log in before you can comment on or make changes to this bug.