Bug 174386 - NavigationAction should track whether the navigation was initiated by the main frame
Summary: NavigationAction should track whether the navigation was initiated by the mai...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Local Build
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords: InRadar
Depends on:
Blocks: 174385
  Show dependency tree / graph
 
Reported: 2017-07-11 13:38 PDT by Daniel Bates
Modified: 2017-07-12 11:02 PDT (History)
6 users (show)

See Also:

Attachments
Patch (24.46 KB, patch)
2017-07-11 13:41 PDT, Daniel Bates 		beidson: review+
Details | Formatted Diff | Diff
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 Daniel Bates 2017-07-11 13:38:49 PDT 
Towards fixing bug #174385, we make NavigationAction actually track whether the navigation was initiated by the main frame.
Comment 1 Daniel Bates 2017-07-11 13:41:12 PDT 
Created attachment 315157 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2017-07-11 13:44:16 PDT 
<rdar://problem/33245267>
Comment 3 Daniel Bates 2017-07-12 09:32:57 PDT 
Comment on attachment 315157 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        initiated by the main frame in r219170 is not possible to initialize this state

"is not" => "it is not"

> Source/WebCore/ChangeLog:12
> +        when asking the embedding client whether to allow a navigation. We will make us

us => use

> Source/WebCore/ChangeLog:13
> +        of it in a subsequent commit.

"in a subsequent commit" => "in the fix for <https://bugs.webkit.org/show_bug.cgi?id=174385>"
Comment 4 Brady Eidson 2017-07-12 09:38:39 PDT 
Comment on attachment 315157 [details]
Patch

In some places in this patch you change names:
e.g. m_navigationInitiatedByMainFrame becomes m_initiatedByMainFrame

I'm not sure why when the type remains "NavigationInitiatedByMainFrame"

I do think this flag of "initiated by main frame" has way more potential uses besides navigation, so the name changes actually kind of make sense.

But please either change the typename as well, or leave the variable/method names for now.
Comment 5 Daniel Bates 2017-07-12 10:10:25 PDT 
(In reply to Brady Eidson from comment #4)
> Comment on attachment 315157 [details]
> Patch
> 
> In some places in this patch you change names:
> e.g. m_navigationInitiatedByMainFrame becomes m_initiatedByMainFrame
> 
> I'm not sure why when the type remains "NavigationInitiatedByMainFrame"
> 

Filed bug #174427 to rename NavigationInitiatedByMainFrame to InitiatedByMainFrame.
Comment 6 Daniel Bates 2017-07-12 10:25:45 PDT 
Committed r219407: <http://trac.webkit.org/changeset/219407>
Comment 7 Daniel Bates 2017-07-12 11:02:30 PDT 
Fix bad merge of patch after <http://trac.webkit.org/changeset/219404> (bug #174427): <http://trac.webkit.org/changeset/219410>.