Bug 42479 – [Chromium] Implement WebFormElement::userSubmitted().

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Hawkins]

[Chromium] Implement WebFormElement::userSubmitted().

[James Hawkins]

Created attachment 61843 [details]
Patch

[David Holloway]

LGTM.

[Jay Civelli]

LGTM

[Kent Tamura]

Comment on attachment 61843 [details]
Patch

Could you explain why we need this change in ChangeLog please?

[Darin Fisher (:fishd, Google)]

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?

[James Hawkins]

(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.

[James Hawkins]

(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.

[James Hawkins]

Created attachment 62018 [details]
Patch

[Darin Fisher (:fishd, Google)]

(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

[James Hawkins]

Created attachment 62105 [details]
Patch

[James Hawkins]

(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

[James Hawkins]

Committed r63786: <http://trac.webkit.org/changeset/63786>