Summary: | [Chromium] Implement WebFormElement::userSubmitted(). | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | James Hawkins <jhawkins> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | dglazkov, dhollowa, fishd, jcivelli, levin | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Other | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
James Hawkins
2010-07-16 13:33:19 PDT
Created attachment 61843 [details]
Patch
LGTM. LGTM Comment on attachment 61843 [details]
Patch
Could you explain why we need this change in ChangeLog please?
Comment on attachment 61843 [details]
Patch
WebCore/html/HTMLFormElement.cpp:409
+ // common browsers( sick! ) allow it be canceled.
both spellings are correct according to the dictionary. probably
better to leave the existing code unchanged.
WebKit/chromium/src/WebFormElement.cpp:71
+ return constUnwrap<HTMLFormElement>()->submissionTrigger() == NotSubmittedByJavaScript;
are you sure you don't want to expose an enum for the variety
of submission trigger types?
WebKit/chromium/src/WebFormElement.cpp:69
+ bool WebFormElement::userSubmitted() const
I'm having a hard time understanding what this method name implies. Is
this telling me that the user already submitted this form? what happens
if the form is submitted multiple times (e.g., suppose it has a target
that corresponds to an IFRAME)? what if the form is submitted once by
javascript and then once by user action?
what if the user clicks on a link, and some javascript runs, which
then calls form.submit()? would that be regarded as a non-user
submitted form?
(In reply to comment #4) > (From update of attachment 61843 [details]) > Could you explain why we need this change in ChangeLog please? Added more info to the ChangeLog. (In reply to comment #5) > (From update of attachment 61843 [details]) > WebCore/html/HTMLFormElement.cpp:409 > + // common browsers( sick! ) allow it be canceled. > both spellings are correct according to the dictionary. probably > better to leave the existing code unchanged. > Done. > WebKit/chromium/src/WebFormElement.cpp:71 > + return constUnwrap<HTMLFormElement>()->submissionTrigger() == NotSubmittedByJavaScript; > are you sure you don't want to expose an enum for the variety > of submission trigger types? > I thought about it, but decided not to over-engineer it for now (keeping it simple). If a client needs to know more than just whether a user submitted the form, it's simple to add. > WebKit/chromium/src/WebFormElement.cpp:69 > + bool WebFormElement::userSubmitted() const > I'm having a hard time understanding what this method name implies. Is > this telling me that the user already submitted this form? what happens > if the form is submitted multiple times (e.g., suppose it has a target > that corresponds to an IFRAME)? what if the form is submitted once by > javascript and then once by user action? > What do you think about wasUserSubmitted()? The value returned is for the last submitted form, so in the case you mention, if the client calls this method after the user submitted the form, it will return true. > what if the user clicks on a link, and some javascript runs, which > then calls form.submit()? would that be regarded as a non-user > submitted form? From my reading of the code, this latter case will not be consider user-submitted. Created attachment 62018 [details]
Patch
(In reply to comment #7) > I thought about it, but decided not to over-engineer it for now (keeping it > simple). If a client needs to know more than just whether a user submitted > the form, it's simple to add. OK > What do you think about wasUserSubmitted()? The value returned is for the last > submitted form, so in the case you mention, if the client calls this method > after the user submitted the form, it will return true. Sounds better. The verb helps. > > what if the user clicks on a link, and some javascript runs, which > > then calls form.submit()? would that be regarded as a non-user > > submitted form? > > From my reading of the code, this latter case will not be consider user- > submitted. OK Created attachment 62105 [details]
Patch
(In reply to comment #9) > (In reply to comment #7) > > > What do you think about wasUserSubmitted()? The value returned is for the last > > submitted form, so in the case you mention, if the client calls this method > > after the user submitted the form, it will return true. > > Sounds better. The verb helps. > Done Committed r63786: <http://trac.webkit.org/changeset/63786> |