[Chromium] Implement WebFormElement::userSubmitted().
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>