Bug 26211 - Cancel in onbeforeunload dialog sometime causes a button to stop working.
Summary: Cancel in onbeforeunload dialog sometime causes a button to stop working.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 525.x (Safari 3.2)
Hardware: PC Windows XP
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: HasReduction, InRadar
: 66128 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-06-05 01:48 PDT by Tore B. Krudtaa
Modified: 2011-08-12 12:57 PDT (History)
5 users (show)

See Also:


Attachments
Testcase (2.21 KB, text/html)
2009-06-05 01:50 PDT, Tore B. Krudtaa
no flags Details
Patch (24.20 KB, patch)
2011-08-12 12:27 PDT, Andy Estes
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tore B. Krudtaa 2009-06-05 01:48:14 PDT
If you (have a form and) are using the onbeforeunload
Then you have a button which run code like this:
document.frmName.submit();
which causes the onbeforeunload to trigger

If you then press "Cancel" (Safari) or "Stay on this page" (Chrome) 
then, if you try clicking the same button again, nothing happens.

Now, if you have a textfield on the same page, you could mouse-click on the text field to put cursor in textfield and then TAB out of the tetfield (using TAB key) and the button will start working again.

(If you just mouse-click inside the textfield and then mouse-click outside the textfield again, then button will not start working again.)

Please see testcase to see how this works.
Comment 1 Tore B. Krudtaa 2009-06-05 01:50:26 PDT
Created attachment 30998 [details]
Testcase

Use this to reproduce the bug
Comment 2 Tore B. Krudtaa 2009-06-09 05:33:09 PDT
This bug is also in 

Safari for win, version: 4.0 (530.17)

and latest version of Chrome... version: 2.0.172.30
Comment 3 Steve Waring 2010-01-04 14:58:14 PST
Further to this. If you have a number of elements with an associated onclick function on the page, where by the function does some work and then submits the form, then:

When you click on one of the elements, the leave/stay fialogue is shown. If you choose stay on page, then thereafter clicking on the element you clicked on previously does not give you the leave/stay dialogue, and the page is not unloaded. IE the same bug as described above. If you now click on a different element that has the same function bound to the onclick event, the same scenario is repeated, but the first element you clicked on now "comes back to life" and clicking on it once again gives you the leave/stay dialogue.

In this situation, clicking into a text box does not bring the onclick element back into life.

This happens in Google Chrome 3.0.195.38
Comment 4 Andy Estes 2011-08-12 12:27:29 PDT
Created attachment 103793 [details]
Patch
Comment 5 Andy Estes 2011-08-12 12:28:10 PDT
*** Bug 66128 has been marked as a duplicate of this bug. ***
Comment 6 Andy Estes 2011-08-12 12:29:12 PDT
<rdar://problem/9765500>
Comment 7 Alexey Proskuryakov 2011-08-12 12:39:25 PDT
Comment on attachment 103793 [details]
Patch

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

I only glanced over the patch, since Jessie had a deeper look already.

> LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html:22
> +    return "";

It might be slightly less confusing to return an actual string like "Please don't agree to closing the page to test". In fact, does this test work for manual testing? It seems like it should.

> Source/WebCore/loader/FrameLoader.cpp:2736
> +    if (!shouldClose)
> +         m_submittedFormURL = KURL();

Bad indentation here (five spaces).
Comment 8 Andy Estes 2011-08-12 12:42:51 PDT
(In reply to comment #7)
> (From update of attachment 103793 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103793&action=review
> 
> I only glanced over the patch, since Jessie had a deeper look already.
> 
> > LayoutTests/fast/loader/form-submission-after-beforeunload-cancel.html:22
> > +    return "";
> 
> It might be slightly less confusing to return an actual string like "Please don't agree to closing the page to test". In fact, does this test work for manual testing? It seems like it should.

Agree. It does work for manual testing. I have a paragraph of text in the test explaining what to do, but having useful text in the confirmation dialog will make it even more obvious.

> 
> > Source/WebCore/loader/FrameLoader.cpp:2736
> > +    if (!shouldClose)
> > +         m_submittedFormURL = KURL();
> 
> Bad indentation here (five spaces).

I blame Xcode :(
Comment 9 Andy Estes 2011-08-12 12:57:04 PDT
Committed r92982: <http://trac.webkit.org/changeset/92982>