<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body><span class="vcard"><a class="email" href="mailto:bburg@apple.com" title="Brian Burg <bburg@apple.com>"> <span class="fn">Brian Burg</span></a>
</span> changed
          <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - WebDriver: wait until navigation is complete before running new commands and after a click"
   href="https://bugs.webkit.org/show_bug.cgi?id=174670">bug 174670</a>
          <br>
             <table border="1" cellspacing="0" cellpadding="8">
          <tr>
            <th>What</th>
            <th>Removed</th>
            <th>Added</th>
          </tr>

         <tr>
           <td style="text-align:right;">Attachment #315976 Flags</td>
           <td>review?
           </td>
           <td>review+
           </td>
         </tr></table>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - WebDriver: wait until navigation is complete before running new commands and after a click"
   href="https://bugs.webkit.org/show_bug.cgi?id=174670#c9">Comment # 9</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - WebDriver: wait until navigation is complete before running new commands and after a click"
   href="https://bugs.webkit.org/show_bug.cgi?id=174670">bug 174670</a>
              from <span class="vcard"><a class="email" href="mailto:bburg@apple.com" title="Brian Burg <bburg@apple.com>"> <span class="fn">Brian Burg</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=315976&action=diff" name="attach_315976" title="Patch">attachment 315976</a> <a href="attachment.cgi?id=315976&action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=315976&action=review">https://bugs.webkit.org/attachment.cgi?id=315976&action=review</a>

r=me with a few questions. Great work!

<span class="quote">> Source/WebDriver/ChangeLog:15
> +        6.3 Processing Model. Step 7. Wait for navigation to complete. If this returns an error return its value and</span >

Nit: step 6

This suggests that we should be waiting for navigations to complete after any command. Should this be extracted out of particular Session methods, then?

<span class="quote">> Source/WebDriver/ChangeLog:19
> +        14.1 Element Click. Step 10. If the click causes navigation: 1. Run the post-navigation checks and return its</span >

I don't understand substep 1. How could the post-navigation checks do anything useful if the click just initiated a load? Ostensibly, the load has likely not been sent out yet, or hasn't received a response in the time waited during Step 9.

<span class="quote">> Source/WebDriver/Session.cpp:947
> +    if (!m_toplevelBrowsingContext) {</span >

Nit: add quotations from specific sections/steps of the spec, especially for error cases.

<span class="quote">> Source/WebDriver/Session.cpp:954
> +    if (m_browsingContext)</span >

Can we rename this local to m_selectedFrameBrowsingContext or something similar? Otherwise it's a bit obscure why there are two browsing context members.

<span class="quote">> Source/WebKit/ChangeLog:24
> +        all the frames. Complete page operations of it's a main frame, or frame operations otherwise.</span >

Nit: of->if

<span class="quote">> Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp:467
> +        if (auto callback = m_pendingNavigationInBrowsingContextCallbacksPerPage.take(frame.page()->pageID()))</span >

When asked to wait on a page navigation, couldn't we use the main frame as the callback key and remove m_pendingNavigationInBrowsingContextCallbacksPerPage? Or does the frame somehow not survive long enough?</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>