[Webkit-unassigned] [Bug 178720] [GTK][WPE] Switch to use API::NavigationClient

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=178720

--- Comment #9 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 324800
  --> https://bugs.webkit.org/attachment.cgi?id=324800
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=324800&action=review

These aren't very important.  The important thing is that API::NavigationClient is used.  This patch is a huge step in a good direction.

>>> Source/WebKit/UIProcess/API/APINavigationAction.h:44
>>> +    static Ref<NavigationAction> create(WebKit::NavigationActionData&& navigationActionData, API::FrameInfo* sourceFrame, std::optional<WTF::String> targetFrameName, WebCore::ResourceRequest&& request, bool shouldOpenAppLinks, RefPtr<UserInitiatedAction>&& userInitiatedAction)
>> 
>> I think we should just have one create function.
> 
> Why? targetFrame and targetFrameName are mutually exclusive.

If someone in the future is deciding which create function to use or modify, having one create function will be simpler.  They will just use nullptr/nullopt/{ } for the fields that don't apply rather than wondering what else is different.

>>> Source/WebKit/UIProcess/API/APINavigationAction.h:85
>>> +    std::optional<WTF::String> m_targetFrameName;
>> 
>> Could we have a null string meaning there's no frame name instead of an optional string?
> 
> Yes, sure, I guess we just need to decide which way is preferred in WebKit in general, because other reviewers have suggested the opposite in other patches.

I don't really care in this case.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171108/f454255c/attachment.html>