Skip to content

Web Extensions: Parsing for Create() Bookmark function #47248

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
Merged
merged 1 commit into from
Jun 30, 2025
Merged

Web Extensions: Parsing for Create() Bookmark function #47248

merged 1 commit into from
Jun 30, 2025
+102 −13

Conversation

akutira-umich
Copy link
Contributor

@akutira-umich akutira-umich commented Jun 26, 2025 •
edited by webkit-early-warning-system
Loading

7c3bccb

Web Extensions: Parsing for Create() Bookmark function
rdar://154231013
https://bugs.webkit.org/show_bug.cgi?id=294932

Reviewed by Timothy Hatcher.

Parses the create function using a CreateDetails Object which takes in the parameters
for the dictionary. For now this info from the dictionary is put into MockBookmarkNodes.
Also makes sure default type of a bookmark create request is assigned properly and
dateAdded is a parameter you can set only for testing purposes.
Created tests for these as well.

* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm:
(WebKit::toWebAPI):
(WebKit::toTypeImpl):
(WebKit::WebExtensionAPIBookmarks::createBookmark):
(WebKit::WebExtensionAPIBookmarks::getRecent):
(WebKit::WebExtensionAPIBookmarks::createDictionaryFromNode):
* Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIBookmarks.mm:
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPICreateParse)):

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

2092a9f

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on previous version of this PR (hash e2a9977)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 win
✅ 🧪 bindings ⏳ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1   🛠 wpe-cairo
⏳ 🛠 🧪 jsc ⏳ 🧪 api-ios   🧪 mac-wk2 ⏳ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision   🧪 mac-AS-debug-wk2 ⏳ 🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-wk2-stress ⏳ 🧪 api-gtk
  🧪 vision-wk2   🧪 mac-intel-wk2 ⏳ 🛠 playstation
⏳ 🛠 tv ❌ 🛠 mac-safer-cpp ⏳ 🛠 jsc-armv7
⏳ 🛠 tv-sim ⏳ 🧪 jsc-armv7-tests
⏳ 🛠 watch
  🛠 watch-sim

@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parsing-for-Create-and-Move-Bookmark-function branch from e2a9977 to 47efa34 Compare June 26, 2025 17:40
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 47efa34)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ⏳ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parsing-for-Create-and-Move-Bookmark-function branch from 47efa34 to cc2a9e2 Compare June 26, 2025 22:24
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on previous version of this PR (hash cc2a9e2)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
❌ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

xeenon
xeenon requested changes Jun 26, 2025
View reviewed changes
@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parsing-for-Create-and-Move-Bookmark-function branch from cc2a9e2 to 9b87032 Compare June 27, 2025 21:41
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 27, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 9b87032)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

xeenon
xeenon requested changes Jun 28, 2025
View reviewed changes
Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated

static NSDictionary *toWebAPI(const WebExtensionAPIBookmarks::MockBookmarkNode& node)
{
NSString* typeString = toWebAPI(node.type, node.url.createNSString().get());
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
NSString* typeString = toWebAPI(node.type, node.url.createNSString().get());

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
dateAddedKey: @(node.dateAdded.secondsSinceEpoch().milliseconds()),
typeKey: bookmarkKey
typeKey: typeString
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
typeKey: typeString
typeKey: toWebAPI(node.type, node.url.createNSString().get());

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
@@ -67,15 +108,32 @@
if (!validateDictionary(bookmark, @"bookmark", requiredKeys, types, outExceptionString))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!validateDictionary(bookmark, @"bookmark", requiredKeys, types, outExceptionString))
if (!validateDictionary(bookmark, @"bookmark", nil, types, outExceptionString))

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
@@ -67,15 +108,32 @@
if (!validateDictionary(bookmark, @"bookmark", requiredKeys, types, outExceptionString))
return;

parsedDetails.index = objectForKey<NSNumber>(bookmark, indexKey).intValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
parsedDetails.index = objectForKey<NSNumber>(bookmark, indexKey).intValue;
parsedDetails.index = objectForKey<NSNumber>(bookmark, indexKey).unsignedLongValue;

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
parsedDetails.url = objectForKey<NSString>(bookmark, urlKey);

if (bookmark[typeKey]) {
std::optional<WebExtensionAPIBookmarks::BookmarkTreeNodeType> parsedType = toImpl(dynamic_objc_cast<NSString>(bookmark[typeKey]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::optional<WebExtensionAPIBookmarks::BookmarkTreeNodeType> parsedType = toImpl(dynamic_objc_cast<NSString>(bookmark[typeKey]));
auto parsedType = toTypeImpl(dynamic_objc_cast<NSString>(bookmark[typeKey]));

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
return tempNode;
}

static std::optional<WebExtensionAPIBookmarks::BookmarkTreeNodeType> toImpl(NSString *typeString)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static std::optional<WebExtensionAPIBookmarks::BookmarkTreeNodeType> toImpl(NSString *typeString)
static std::optional<WebExtensionAPIBookmarks::BookmarkTreeNodeType> toTypeImpl(NSString *typeString)

Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm Outdated
Comment on lines 118 to 117
if (!parsedType)
*outExceptionString = toErrorString(nullString(), typeKey, @"it must specify either 'bookmark' or 'folder'").createNSString().autorelease();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!parsedType)
*outExceptionString = toErrorString(nullString(), typeKey, @"it must specify either 'bookmark' or 'folder'").createNSString().autorelease();
if (!parsedType) {
*outExceptionString = toErrorString(nullString(), typeKey, @"it must specify either 'bookmark' or 'folder'").createNSString().autorelease();
return;
}

Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h Outdated
private:

public:
enum class BookmarkTreeNodeType {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
enum class BookmarkTreeNodeType {
enum class BookmarkTreeNodeType : uint_8_t {

Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h Outdated
};

struct CreateDetails {
std::optional<int> index;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
std::optional<int> index;
std::optional<size_t> index;

@akutira-umich akutira-umich force-pushed the eng/Web-Extensions-Parsing-for-Create-and-Move-Bookmark-function branch from 9b87032 to 2092a9f Compare June 30, 2025 16:48
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash 2092a9f)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@xeenon xeenon added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing WebKit Extensions Bugs related to extension support. labels Jun 30, 2025
@akutira-umich @xeenon
Web Extensions: Parsing for Create() Bookmark function
7c3bccb 
rdar://154231013
https://bugs.webkit.org/show_bug.cgi?id=294932

Reviewed by Timothy Hatcher.

Parses the create function using a CreateDetails Object which takes in the parameters
for the dictionary. For now this info from the dictionary is put into MockBookmarkNodes.
Also makes sure default type of a bookmark create request is assigned properly and
dateAdded is a parameter you can set only for testing purposes.
Created tests for these as well.

* Source/WebKit/WebProcess/Extensions/API/Cocoa/WebExtensionAPIBookmarksCocoa.mm:
(WebKit::toWebAPI):
(WebKit::toTypeImpl):
(WebKit::WebExtensionAPIBookmarks::createBookmark):
(WebKit::WebExtensionAPIBookmarks::getRecent):
(WebKit::WebExtensionAPIBookmarks::createDictionaryFromNode):
* Source/WebKit/WebProcess/Extensions/API/WebExtensionAPIBookmarks.h:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIBookmarks.mm:
(TestWebKitAPI::TEST_F(WKWebExtensionAPIBookmarks, BookmarksAPICreateParse)):

Canonical link: https://commits.webkit.org/296824@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Web-Extensions-Parsing-for-Create-and-Move-Bookmark-function branch from 2092a9f to 7c3bccb Compare June 30, 2025 20:24
@webkit-commit-queue
Copy link
Collaborator

Committed 296824@main (7c3bccb): https://commits.webkit.org/296824@main

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

@webkit-commit-queue webkit-commit-queue merged commit 7c3bccb into WebKit:main Jun 30, 2025
@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 Jun 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AriYoung00 AriYoung00 AriYoung00 left review comments

@b-weinstein b-weinstein b-weinstein left review comments

@xeenon xeenon xeenon approved these changes

Assignees
No one assigned
Labels
WebKit Extensions Bugs related to extension support.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@akutira-umich @webkit-early-warning-system @webkit-commit-queue @xeenon @AriYoung00 @b-weinstein