WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
236245
Add settings to restrict media containers and codecs when in Captive Portal mode
https://bugs.webkit.org/show_bug.cgi?id=236245
Summary
Add settings to restrict media containers and codecs when in Captive Portal mode
Jer Noble
Reported
2022-02-07 08:51:15 PST
Add settings to restrict media containers and codecs when in Captive Portal mode
Attachments
Patch
(88.69 KB, patch)
2022-02-07 10:48 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(86.37 KB, patch)
2022-02-08 08:52 PST
,
Jer Noble
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(86.40 KB, patch)
2022-02-08 09:49 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(82.54 KB, patch)
2022-02-08 10:01 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch
(86.36 KB, patch)
2022-02-08 10:38 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(86.37 KB, patch)
2022-02-11 09:42 PST
,
Jer Noble
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch for landing
(88.63 KB, patch)
2022-02-11 18:04 PST
,
Jer Noble
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch for landing
(88.63 KB, patch)
2022-02-11 18:14 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Patch for landing
(89.30 KB, patch)
2022-02-12 09:05 PST
,
Jer Noble
no flags
Details
Formatted Diff
Diff
Show Obsolete
(8)
View All
Add attachment
proposed patch, testcase, etc.
Jer Noble
Comment 1
2022-02-07 10:48:02 PST
Created
attachment 451125
[details]
Patch
Peng Liu
Comment 2
2022-02-07 13:15:41 PST
Comment on
attachment 451125
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451125&action=review
> Source/WTF/Scripts/Preferences/WebPreferences.yaml:1329 > +MediaAudioCodecIDsAllowedWhenCaptivePortalModeEnabled:
The name looks a little strange. Is "MediaAudioCodecIDsAllowedInCaptivePortalMode" better?
> Source/WebCore/page/SettingsBase.cpp:238 > + Vector<FourCC> newTypes; > + for (auto type : StringView(types).split(',')) {
Nit. Maybe we can implement this in a function and reuse the function?
> Source/WebKit/WebKit.xcodeproj/xcshareddata/xcschemes/WebKitSwift.xcscheme:1 > +<?xml version="1.0" encoding="UTF-8"?>
Is this change relevant?
Eric Carlson
Comment 3
2022-02-07 14:49:29 PST
Comment on
attachment 451125
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451125&action=review
r=me once the bots are happy
> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:362 > + auto& settings = document().settings(); > + if (!contentTypeMeetsContainerAndCodecTypeRequirements(contentType, settings.allowedMediaContainerTypes(), settings.allowedMediaCodecTypes())) > + return Exception { NotSupportedError };
You could add a public method to MediaSource instead of repeating the check here.
Jer Noble
Comment 4
2022-02-07 23:50:38 PST
(In reply to Peng Liu from
comment #2
)
> Comment on
attachment 451125
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=451125&action=review
> > > Source/WTF/Scripts/Preferences/WebPreferences.yaml:1329 > > +MediaAudioCodecIDsAllowedWhenCaptivePortalModeEnabled: > > The name looks a little strange. Is > "MediaAudioCodecIDsAllowedInCaptivePortalMode" better?
Ok!
> > Source/WebCore/page/SettingsBase.cpp:238 > > + Vector<FourCC> newTypes; > > + for (auto type : StringView(types).split(',')) { > > Nit. Maybe we can implement this in a function and reuse the function?
I dunno, it's pretty concise as it is.
> > Source/WebKit/WebKit.xcodeproj/xcshareddata/xcschemes/WebKitSwift.xcscheme:1 > > +<?xml version="1.0" encoding="UTF-8"?> > > Is this change relevant?
Whoops! Nope. (In reply to Eric Carlson from
comment #3
)
> Comment on
attachment 451125
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=451125&action=review
> > r=me once the bots are happy > > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:362 > > + auto& settings = document().settings(); > > + if (!contentTypeMeetsContainerAndCodecTypeRequirements(contentType, settings.allowedMediaContainerTypes(), settings.allowedMediaCodecTypes())) > > + return Exception { NotSupportedError }; > > You could add a public method to MediaSource instead of repeating the check > here.
It's already been factored into a single utility method; I'm not sure it's worth further refactoring.
Jer Noble
Comment 5
2022-02-08 08:52:21 PST
Created
attachment 451255
[details]
Patch
Jer Noble
Comment 6
2022-02-08 09:49:23 PST
Created
attachment 451264
[details]
Patch
Jer Noble
Comment 7
2022-02-08 10:01:03 PST
Created
attachment 451267
[details]
Patch
Sam Weinig
Comment 8
2022-02-08 10:05:08 PST
Comment on
attachment 451267
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451267&action=review
> Source/WTF/ChangeLog:3 > + [Cocoa] Adopt -streamDataParser:didProvideContentKeySpecifier:forTrackID: delegate callback
Is the right change for this bug? The title is quite different.
Jer Noble
Comment 9
2022-02-08 10:34:06 PST
(In reply to Sam Weinig from
comment #8
)
> Comment on
attachment 451267
[details]
> Patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=451267&action=review
> > > Source/WTF/ChangeLog:3 > > + [Cocoa] Adopt -streamDataParser:didProvideContentKeySpecifier:forTrackID: delegate callback > > Is the right change for this bug? The title is quite different.
Ugh. webkit-patch uploaded that patch to the wrong bug. Will fix.
Jer Noble
Comment 10
2022-02-08 10:38:53 PST
Created
attachment 451271
[details]
Patch
Eric Carlson
Comment 11
2022-02-08 10:55:05 PST
Comment on
attachment 451271
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451271&action=review
> Source/WTF/ChangeLog:134 > +2022-02-07 Jer Noble <
jer.noble@apple.com
> > + > + Add settings to restrict media containers and codecs when in Captive Portal mode > +
https://bugs.webkit.org/show_bug.cgi?id=236245
> + > + Reviewed by NOBODY (OOPS!). > + > + * Scripts/Preferences/WebPreferences.yaml: > +
Nit: move this to the top of the ChangeLog
> LayoutTests/media/video-test.js:243 > + return waitFor(element, type, false, true);
s/false, true/false, false/
Peng Liu
Comment 12
2022-02-08 11:35:34 PST
Comment on
attachment 451271
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=451271&action=review
> LayoutTests/ChangeLog:373 > +2022-02-07 Jer Noble <
jer.noble@apple.com
>
Nit: move this to the top of the ChangeLog. :-)
Jer Noble
Comment 13
2022-02-11 09:42:58 PST
Created
attachment 451713
[details]
Patch for landing
Jer Noble
Comment 14
2022-02-11 13:46:11 PST
Looks like settings are bleeding between runs. Will update with a patch that resets these new settings to consistent values between runs.
Jer Noble
Comment 15
2022-02-11 18:04:35 PST
Created
attachment 451764
[details]
Patch for landing
Jer Noble
Comment 16
2022-02-11 18:14:28 PST
Created
attachment 451766
[details]
Patch for landing
Jer Noble
Comment 17
2022-02-12 09:05:36 PST
Created
attachment 451785
[details]
Patch for landing
EWS
Comment 18
2022-02-12 10:18:04 PST
Committed
r289697
(
247182@main
): <
https://commits.webkit.org/247182@main
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 451785
[details]
.
Radar WebKit Bug Importer
Comment 19
2022-02-12 10:19:20 PST
<
rdar://problem/88857365
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug