Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify MiniBrowser settings menu construction #5992

Merged
merged 1 commit into from Nov 1, 2022
Merged

Simplify MiniBrowser settings menu construction #5992

merged 1 commit into from Nov 1, 2022

Conversation

hortont424
Copy link
Contributor

@hortont424 hortont424 commented Nov 1, 2022 •
edited by webkit-early-warning-system

d8123ac

Simplify MiniBrowser settings menu construction
https://bugs.webkit.org/show_bug.cgi?id=247305

Reviewed by Simon Fraser.

* Tools/MiniBrowser/mac/SettingsController.m:
(-[SettingsController initWithMenu:]):
(addItemToMenuWithTarget):
(addSeparatorToMenu):
(addSubmenuToMenu):
(-[SettingsController _populateMenu:]):
(-[SettingsController buildUserAgentsMenu:]):
(-[SettingsController validateMenuItem:]):
(-[SettingsController changeCustomUserAgent:]):
(-[SettingsController dealloc]): Deleted.
(-[SettingsController _addItemWithTitle:action:indented:]): Deleted.
(-[SettingsController _addHeaderWithTitle:]): Deleted.
(-[SettingsController _populateMenu]): Deleted.
(-[SettingsController changeCutomUserAgent:]): Deleted.
Simplify the menu construction with some more helper functions.
Move the resource usage overlays into the overlay submenu.
Remove the empty WebKit1-only settings section.
Fix some typos.

Canonical link: https://commits.webkit.org/256195@main

9dc15e8

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
  🧪 api-ios   🧪 api-mac ❌ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk1
✅ 🛠 tv-sim ❌ 🧪 mac-wk2
❌ 🛠 🧪 merge ✅ 🛠 watch   🧪 mac-AS-debug-wk2
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@hortont424 hortont424 self-assigned this Nov 1, 2022
@hortont424 hortont424 added Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases WebKit Local Build labels Nov 1, 2022
@hortont424 hortont424 requested a review from smfr November 1, 2022 05:22
@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 9dc15e8)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
  🧪 api-ios   🧪 api-mac ❌ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-wk1
✅ 🛠 tv-sim ❌ 🧪 mac-wk2
❌ 🛠 🧪 merge ✅ 🛠 watch   🧪 mac-AS-debug-wk2
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 1, 2022
smfr
smfr approved these changes Nov 1, 2022
View reviewed changes
Tools/MiniBrowser/mac/SettingsController.m
{
[_menu addItem:[NSMenuItem separatorItem]];
[_menu addItem:[[[NSMenuItem alloc] initWithTitle:title action:nil keyEquivalent:@""] autorelease]];
NSMenuItem *submenuItem = [[[NSMenuItem alloc] initWithTitle:title action:nil keyEquivalent:@""] autorelease];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to ARCify MiniBrowser code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, may do that at some point.

@hortont424 hortont424 added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Nov 1, 2022
@webkit-commit-queue webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Nov 1, 2022
@hortont424 hortont424 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Nov 1, 2022
@hortont424
Simplify MiniBrowser settings menu construction
d8123ac 
https://bugs.webkit.org/show_bug.cgi?id=247305

Reviewed by Simon Fraser.

* Tools/MiniBrowser/mac/SettingsController.m:
(-[SettingsController initWithMenu:]):
(addItemToMenuWithTarget):
(addSeparatorToMenu):
(addSubmenuToMenu):
(-[SettingsController _populateMenu:]):
(-[SettingsController buildUserAgentsMenu:]):
(-[SettingsController validateMenuItem:]):
(-[SettingsController changeCustomUserAgent:]):
(-[SettingsController dealloc]): Deleted.
(-[SettingsController _addItemWithTitle:action:indented:]): Deleted.
(-[SettingsController _addHeaderWithTitle:]): Deleted.
(-[SettingsController _populateMenu]): Deleted.
(-[SettingsController changeCutomUserAgent:]): Deleted.
Simplify the menu construction with some more helper functions.
Move the resource usage overlays into the overlay submenu.
Remove the empty WebKit1-only settings section.
Fix some typos.

Canonical link: https://commits.webkit.org/256195@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Simplify-MiniBrowser-settings-menu-construction branch from 9dc15e8 to d8123ac Compare November 1, 2022 19:37
@webkit-early-warning-system webkit-early-warning-system merged commit d8123ac into WebKit:main Nov 1, 2022
@webkit-commit-queue
Copy link
Collaborator

Committed 256195@main (d8123ac): https://commits.webkit.org/256195@main

Reviewed commits have been landed. Closing PR #5992 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smfr smfr smfr approved these changes

@JonWBedard JonWBedard Awaiting requested review from JonWBedard JonWBedard is a code owner

Assignees

@hortont424 hortont424

Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
Milestone
No milestone
5 participants
@hortont424 @webkit-early-warning-system @webkit-commit-queue @smfr @webkit-ews-buildbot