Bug 214955 - Remove unused JS and CSS files of media controls
Summary: Remove unused JS and CSS files of media controls
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Devin Rousso
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-29 18:28 PDT by Peng Liu
Modified: 2021-03-24 22:44 PDT (History)
38 users (show)

See Also:

Attachments
Patch (346.55 KB, patch)
2020-07-29 18:30 PDT, Peng Liu 		no flags Details | Formatted Diff | Diff
Patch (211.65 KB, patch)
2020-07-29 18:37 PDT, Peng Liu 		dbates: review+
Details | Formatted Diff | Diff
[Patch] WIP (1.21 MB, patch)
2021-03-17 19:46 PDT, Devin Rousso 		hi: commit-queue-
Details | Formatted Diff | Diff
[Patch] WIP (1.21 MB, patch)
2021-03-17 20:01 PDT, Devin Rousso 		hi: commit-queue-
Details | Formatted Diff | Diff
[Patch] WIP (1.21 MB, patch)
2021-03-18 00:28 PDT, Devin Rousso 		hi: commit-queue-
Details | Formatted Diff | Diff
Patch (1.25 MB, patch)
2021-03-18 13:59 PDT, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (1.22 MB, patch)
2021-03-19 09:58 PDT, Devin Rousso 		eric.carlson: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (1.23 MB, patch)
2021-03-19 13:37 PDT, Devin Rousso 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (1.23 MB, patch)
2021-03-22 10:13 PDT, Devin Rousso 		no flags Details | Formatted Diff | Diff
Show Obsolete (8) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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