Bug 30491

Summary: Pressing Enter in <isindex> doesn't submit it if there is no form
Product: WebKit Reporter: Mantas <grawity>
Component: FormsAssignee: Daniel Bates <dbates>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, aroben, bdakin, darin, dbates, vicki
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://webblaze.org/dbates/xsstest-isindex.php
Attachments:
Description Flags
Layout tests
none
Patch with test cases (work in progress)
none
Patch with test cases
none
Patch with test cases
darin: review-
Patch with test cases
darin: review+
KeyboardEvent Test none

Description Mantas 2009-10-18 06:03:47 PDT
(I'm not sure if this is the correct place, but...)

I'm using Epiphany 2.28.0 (AppleWebKit/531.2+ libwebkit 1.1.15.2), and I have noticed that it is impossible to submit forms created with the <isindex> HTML tag. The input box appears, but does not react to pressing the Enter key. (No submit button either.)

The same happens with Midori 0.2.0 (same WebKit version).
Comment 1 Alexey Proskuryakov 2009-10-18 12:52:54 PDT
Do you have a test case you could link to, or attach?
Comment 2 Daniel Bates 2009-10-19 00:20:44 PDT
Confirmed this issue effects the Windows and Mac builds.

Example:

http://webblaze.org/dbates/xsstest-isindex.php

Enter a search keyword. Try to the submit the keyword. Neither pressing the return key nor the enter key on the keyboard submit the keyword (as you would expect if this was a typical form).

Expected result:

Without loss of generality, suppose the search keyboard is "dog". Then on pressing return or enter on the keyboard, the page should redirect to: http://webblaze.org/dbates/xsstest-isindex.php?dog
Comment 3 Daniel Bates 2009-10-19 00:43:36 PDT
On looking through the layout tests, the closest test I came across is http/tests/misc/isindex-formdata.html. But this only tests the case when <isindex> is within an HTML form element. All of the other <isindex> layout tests I saw are similar.

Section 7.5 "Queries and Indexes" of the HTML 2.0 spec. (http://www.rfc-editor.org/rfc/rfc1866.txt) implies that the <isindex> element can appear standalone (or at the very least along with an HTML Base element). That is, it does not have to appear within an HTML Form element. Firefox seems to follow this interpretation.
Comment 4 Daniel Bates 2009-10-19 12:17:00 PDT
Created attachment 41437 [details]
Layout tests

Included in this patch are two layout tests, both without enclosing the <isindex> tag within a <form> element:

1) isindex-with-no-form.html: A page with a standalone <isindex> tag.
Any submitted result will be submitted to the same page. This matches the default behavior of Firefox.

2) isindex-with-no-form-base-href.html: A page with a <isindex> tag and <base> tag.
The submitted result should be sent to the page resources/isindex-with-no-form-base-href-submit.html for processing (as indicated by the href property of the HTML Base element).

The tests work by using DRT's eventSender to send the simulate pressing the return key on the keyboard.

Until we fix this bug, both tests time out under DRT.
Comment 5 Daniel Bates 2009-10-24 18:36:11 PDT
Created attachment 41804 [details]
Patch with test cases (work in progress)
Comment 6 Daniel Bates 2009-10-24 20:14:29 PDT
Created attachment 41807 [details]
Patch with test cases
Comment 7 Daniel Bates 2009-10-24 21:16:06 PDT
Comment on attachment 41807 [details]
Patch with test cases

Need to think about this some more. This patch does not conform to <http://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-html#ID-87069980>
Comment 8 Daniel Bates 2009-10-24 22:40:25 PDT
Created attachment 41814 [details]
Patch with test cases

Moved code into HTMLInputElement::defaultEventHandler so that we don't create a form that is visible to the client. Hence, we conform to http://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-html#ID-87069980.

I am open to suggestions on the placement of this code. Also, let me know if you have a better name for the variable myForm.

This patch passes all layout tests, on my Mac, as of r49810.
Comment 9 Daniel Bates 2009-10-24 22:47:54 PDT
Found out that my original patch was basically a revert of:
https://bugs.webkit.org/show_bug.cgi?id=4828 (<rdar://problem/4288307>)

The latest patch <https://bug-30491-attachments.webkit.org/attachment.cgi?id=41814> passes all DRT layout tests, including dom/html/level2/html/HTMLIsIndexElement02.html (which lead to the patch for bug #4828).

I am CC'ing Darin, Viki, and Anders on this bug, since they may have some additional insight (from bug #4828).

Some additional notes:

According to the HTML 2.0 spec (http://www.w3.org/MarkUp/html-spec/html-spec_7.html#SEC7.5), <isindex> should not need to be within a <form> to be able to submit it. Moreover, <isindex> precedes the introduction of HTML forms, see HTML 1.2 <http://www.w3.org/MarkUp/draft-ietf-iiir-html-01.txt>.

Additionally, the 2.0 spec. implies that the href property of the <base> element should be dictate where the value of the <isindex> element is sent  (analogous to the action property of a <form>)(*).

Another issue is that <isindex> should be able to work when inside <head> as stated in the HTML 1.2 and later specs. But we should probably file another bug for this issue.

Both IE 8 and Firefox seem to correctly handle the <isindex> element.
(*) Only Firefox respects this. Doesn't seem to work in IE 8.
Comment 10 Daniel Bates 2009-10-24 23:35:11 PDT
CC'ing Beth Dakin, since she reviewed <http://trac.webkit.org/changeset/10779>.
Comment 11 Darin Adler 2009-10-25 17:45:46 PDT
Comment on attachment 41814 [details]
Patch with test cases

> +        RefPtr<HTMLFormElement> myForm = form();

I don’t think the "my" prefix is helpful to explain this local variable's purpose. By definition, this->form() is "my form", so the prefix simply creates confusion. I suggest formForSubmission as the name here.

> +        bool isIsIndexElementAndThereIsNoForm = (inputType() == ISINDEX) && !form();

It seems a waste to call form() again when we have a local variable with that same value in it. Also, those parentheses are not needed, and we normally omit them although I suppose they could confuse newcomers to C programming.

> +        if (isIsIndexElementAndThereIsNoForm) {
> +            // Since this <isindex> is not contained within a <form>, we create one.
> +            myForm = new HTMLFormElement(formTag, document());
> +            myForm->registerFormElement(this);
> +            myForm->setMethod("GET");
> +            if (!document()->baseURL().isEmpty()) {
> +                // We treat the href property of the <base> element as the form action, as per section 7.5 
> +                // "Queries and Indexes" of the HTML 2.0 spec. <http://www.w3.org/MarkUp/html-spec/html-spec_7.html#SEC7.5>.
> +                myForm->setAction(document()->baseURL().string());
> +            }            
> +        }

Putting so much code in line into this function just for this element is inelegant. I suggest creating a separate function to create the temporary form. It could be a non-member function or if you prefer it could be a private member function named createTemporaryForm which returns a PassRefPtr<HTMLFormElement>. Then the code here would be just:

    RefPtr<HTMLFormElement> formForSubmission = form();

    // If there is no form and the element is an <isindex>, then create a temporary form just to be used for submission.
    if (!formForSubmission && inputType() == ISINDEX)
        formForSubmission = createTemporaryForm(this).

    // Form may never have been present, or may have been destroyed by code responding to the change event.
    if (formForSubmission)
        formForSubmission->submitClick(evt);

I think that's easier to read.

> +        if (isIsIndexElementAndThereIsNoForm)
> +            myForm.clear();

This is not needed. A RefPtr will automatically release its value when it goes out of scope and there's no need to ensure it's dereferenced before calling setDefaultHandled.

Since the <isindex> is one of the form's elements, isn't there a chance this might call dispatchSimulatedClick on the <isindex>? What prevents this from happening? If it does happen, will it do the right thing? Why won't this just end up creating another temporary form in an infinite loop?

Do we guarantee this form won't be seen by any JavaScript code? Is there any way an event handler might somehow see this event and get its target and so get at the underlying form?

> +If you are running this test by hand, submit the phrase: "This is a test" (without quotes).

Doesn't that text automatically get put into the field by the JavaScript even when running the test by hand? I think that instruction is unneeded, but OK.

Why does this need to be an HTTP test? We can do form submissions with local URLs. Can these same tests just be put in fast/forms instead of http/tests/misc and run without requiring the http server? Is it in the http section because of the base element?

This patch seems pretty good and probably ready to go, but I had enough questions and suggestions that I'm going to say review- for now.
Comment 12 Daniel Bates 2009-10-26 17:26:17 PDT
(In reply to comment #11)
> (From update of attachment 41814 [details])
> > +        RefPtr<HTMLFormElement> myForm = form();
> 
> I don’t think the "my" prefix is helpful to explain this local variable's
> purpose. By definition, this->form() is "my form", so the prefix simply creates
> confusion. I suggest formForSubmission as the name here.

Will do.

> 
> > +        bool isIsIndexElementAndThereIsNoForm = (inputType() == ISINDEX) && !form();
> 
> It seems a waste to call form() again when we have a local variable with that
> same value in it. Also, those parentheses are not needed, and we normally omit
> them although I suppose they could confuse newcomers to C programming.
> 
> > +        if (isIsIndexElementAndThereIsNoForm) {
> > +            // Since this <isindex> is not contained within a <form>, we create one.
> > +            myForm = new HTMLFormElement(formTag, document());
> > +            myForm->registerFormElement(this);
> > +            myForm->setMethod("GET");
> > +            if (!document()->baseURL().isEmpty()) {
> > +                // We treat the href property of the <base> element as the form action, as per section 7.5 
> > +                // "Queries and Indexes" of the HTML 2.0 spec. <http://www.w3.org/MarkUp/html-spec/html-spec_7.html#SEC7.5>.
> > +                myForm->setAction(document()->baseURL().string());
> > +            }            
> > +        }
> 
> Putting so much code in line into this function just for this element is
> inelegant. I suggest creating a separate function to create the temporary form.
> It could be a non-member function or if you prefer it could be a private member
> function named createTemporaryForm which returns a PassRefPtr<HTMLFormElement>.
> Then the code here would be just:
> 
>     RefPtr<HTMLFormElement> formForSubmission = form();
> 
>     // If there is no form and the element is an <isindex>, then create a
> temporary form just to be used for submission.
>     if (!formForSubmission && inputType() == ISINDEX)
>         formForSubmission = createTemporaryForm(this).
> 
>     // Form may never have been present, or may have been destroyed by code
> responding to the change event.
>     if (formForSubmission)
>         formForSubmission->submitClick(evt);
> 
> I think that's easier to read.

I agree. I'll refactor it.

> 
> > +        if (isIsIndexElementAndThereIsNoForm)
> > +            myForm.clear();
> 
> This is not needed. A RefPtr will automatically release its value when it goes
> out of scope and there's no need to ensure it's dereferenced before calling
> setDefaultHandled.
> 

Will remove it.

> Since the <isindex> is one of the form's elements, isn't there a chance this
> might call dispatchSimulatedClick on the <isindex>? What prevents this from
> happening? If it does happen, will it do the right thing? Why won't this just
> end up creating another temporary form in an infinite loop?

No, there is no chance this will call dispatchSimulatedClick on the <isindex>.

Looking at HTMLFormElement::submitClick, which is the only place where dispatchSimulatedClick is called from in the file HTMLFormElement.cpp, only image/submit buttons are dispatched simulated clicks by the first conjunction of the inner-if condition (*), element->isSuccessfulSubmitButton(). And, by line 903 of HTMLInputElement.cpp <http://trac.webkit.org/browser/trunk/WebCore/html/HTMLInputElement.cpp#L903>, HTMLInputElement::isSuccessfulSubmitButton() only returns true if it is enabled and the type property is either "image" or "submit". But, an <isindex> element has type "isindex", so (*) never evaluates to true. So, HTMLFormElement::submitClick never calls dispatchSimulatedClick on an <isindex> element.

> Do we guarantee this form won't be seen by any JavaScript code? Is there any
> way an event handler might somehow see this event and get its target and so get
> at the underlying form?

I assume you are referring to the case that dispatchSimulatedClick fired on an <isindex> element, in which case a JavaScript installed event handler may be able to catch the simulated click event. By the above argument, it is impossible for method dispatchSimulatedClick() to be called. So, no simulated click event is generated. Or am I misunderstanding your question?

> > +If you are running this test by hand, submit the phrase: "This is a test" (without quotes).
> 
> Doesn't that text automatically get put into the field by the JavaScript even
> when running the test by hand? I think that instruction is unneeded, but OK.

Changed to read: "If you are running this test by hand, press the enter/return key on your keyboard to submit."

> Why does this need to be an HTTP test? We can do form submissions with local
> URLs. Can these same tests just be put in fast/forms instead of http/tests/misc
> and run without requiring the http server? Is it in the http section because of
> the base element?

Yes, it is in the http section because of the HTML Base element. If you want we can probably separate the test cases so as to put isindex-with-no-form-base-href.html in fast/form and leave isindex-with-no-form-base-href.html in http/tests/misc.

Note, another isindex test case, isindex-formdata.html, (which I based my tests on) is in the http/tests/misc directory.
Comment 13 Darin Adler 2009-10-26 17:28:56 PDT
(In reply to comment #12)
> Yes, it is in the http section because of the HTML Base element. If you want we
> can probably separate the test cases so as to put
> isindex-with-no-form-base-href.html in fast/form and leave
> isindex-with-no-form-base-href.html in http/tests/misc.

I do think that would be better.

> Note, another isindex test case, isindex-formdata.html, (which I based my tests
> on) is in the http/tests/misc directory.

I don't know why. The original author, Adam Roben, might remember. I think it would be better if someone moved it to fast/forms.
Comment 14 Daniel Bates 2009-10-26 17:33:23 PDT
Created attachment 41918 [details]
Patch with test cases

Updated patch based on Darin's suggestions.

I decided to define a no-argument method called HTMLInputElement::createTemporaryFormForIsIndex as opposed to something more generalized (like createTemporaryForm as Darin suggested) because only the <isindex> element needs to create a temporary form and this form must always register the form element we are in (i.e. this) plus possibly use the base href URL as the form action (which is very <isindex>-specific functionality).
Comment 15 Darin Adler 2009-10-26 17:43:13 PDT
Comment on attachment 41918 [details]
Patch with test cases

> +    if (window.eventSender)
> +        eventSender.keyDown(String.fromCharCode(0x0d));

I'm kind of sad that we had to use eventSender to test this. Wasn't there any other way to do it?
Comment 16 Daniel Bates 2009-10-26 19:57:56 PDT
Not that I know of.

The <isindex> element includes no submit button (probably because the submit button did not even exist in the HTML 1.2 spec when <isindex> came out).

So, to submit the <isindex> value you need to press the return/enter key on the keyboard. DRT does not appear to have a general submit form method. Hence I used window.eventSender to simulate a key press.

(In reply to comment #15)
> (From update of attachment 41918 [details])
> > +    if (window.eventSender)
> > +        eventSender.keyDown(String.fromCharCode(0x0d));
> 
> I'm kind of sad that we had to use eventSender to test this. Wasn't there any
> other way to do it?
Comment 17 WebKit Commit Bot 2009-10-26 20:29:34 PDT
Comment on attachment 41918 [details]
Patch with test cases

Rejecting patch 41918 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11526 test cases.
http/tests/misc/isindex-with-no-form.html -> failed

Exiting early after 1 failures. 8619 tests run.
212.12s total testing time

8618 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
4 test cases (<1%) had stderr output
Comment 18 Daniel Bates 2009-10-26 23:05:49 PDT
Oops, I forgot to change isindex-with-no-form-expected.txt to have the line:
"If you are running this test by hand, press the enter/return key on your keyboard to submit."

Will land manually with this correction.

(In reply to comment #17)
> (From update of attachment 41918 [details])
> Rejecting patch 41918 from commit-queue.
> 
> Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari',
> '--quiet', '--exit-after-n-failures=1']" exit_code: 1
> Running build-dumprendertree
> Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
> Testing 11526 test cases.
> http/tests/misc/isindex-with-no-form.html -> failed
> 
> Exiting early after 1 failures. 8619 tests run.
> 212.12s total testing time
> 
> 8618 test cases (99%) succeeded
> 1 test case (<1%) had incorrect layout
> 4 test cases (<1%) had stderr output
Comment 19 Darin Adler 2009-10-27 09:55:19 PDT
(In reply to comment #16)
> The <isindex> element includes no submit button (probably because the submit
> button did not even exist in the HTML 1.2 spec when <isindex> came out).

I don’t understand what you mean by "submit button" here.

> So, to submit the <isindex> value you need to press the return/enter key on the
> keyboard. DRT does not appear to have a general submit form method. Hence I
> used window.eventSender to simulate a key press.

You can simulate the effect of pressing the return/enter key a various ways that do not depend on special features of DumpRenderTree.

For example, you can use DOM keyboard events. An example of a test that uses this technique is forms/input-text-enter.html.

Another possibility that might well work is to call the click() function on the input element.

You should at least try these before landing a test that uses eventSender.
Comment 20 Daniel Bates 2009-10-27 20:36:00 PDT
(In reply to comment #19)
> (In reply to comment #16)
> > The <isindex> element includes no submit button (probably because the submit
> > button did not even exist in the HTML 1.2 spec when <isindex> came out). 
> I don’t understand what you mean by "submit button" here.

Disregarding stylistic markup, you can think of the <isindex> element as semantically equivalent to the HTML markup: <form method="get"><input type="text"/></form>.

Notice, the absence of a submit button (i.e. <input type="submit" .../>). So, how does a user submit such a form? They have to first give <input> focus, then press the return/enter key on their keyboard (at least as far as I know).
 
> > So, to submit the <isindex> value you need to press the return/enter key on the
> > keyboard. DRT does not appear to have a general submit form method. Hence I
> > used window.eventSender to simulate a key press.
> 
> You can simulate the effect of pressing the return/enter key a various ways
> that do not depend on special features of DumpRenderTree.
> 
> For example, you can use DOM keyboard events. An example of a test that uses
> this technique is forms/input-text-enter.html.

I tried to use DOM keyboard events (first thing I tried) to simulate pressing the return/enter key but it didn't work for submitting a form. I'll take a second look.
 
> Another possibility that might well work is to call the click() function on the
> input element.

I did not try this, so I will take a look. But I am sketical since I do not believe clicking on an <input type="text" ...> is sufficient to submit its value. Instead, I would figure it just gives focus to the input.
> 
> You should at least try these before landing a test that uses eventSender.
Comment 21 Darin Adler 2009-10-27 22:15:05 PDT
(In reply to comment #20)
> I tried to use DOM keyboard events (first thing I tried) to simulate pressing
> the return/enter key but it didn't work for submitting a form. I'll take a
> second look.

You should definitely try again; a little debugging should reveal why it's not working. Maybe you sent the wrong type of keyboard event? It's a good idea to start with the input-text-enter.html test because it definitely works and is quite similar to what you're trying to do.
Comment 22 Daniel Bates 2009-10-29 19:36:59 PDT
It appears that it is not working because DOM keyboard events are broken (bug #16735). Moreover, DOM Level 3 Keyboard Events are not working (bug #9933). We need to fix these bugs for us to be able to use DOM keyboard events instead of the DRT eventSender.

On another note, notice that fast/forms/input-text-enter.html also uses DRT eventSender. I suspect this is to work around these bugs.

(In reply to comment #21)
> (In reply to comment #20)
> > I tried to use DOM keyboard events (first thing I tried) to simulate pressing
> > the return/enter key but it didn't work for submitting a form. I'll take a
> > second look.
> 
> You should definitely try again; a little debugging should reveal why it's not
> working. Maybe you sent the wrong type of keyboard event? It's a good idea to
> start with the input-text-enter.html test because it definitely works and is
> quite similar to what you're trying to do.
Comment 23 Daniel Bates 2009-10-29 19:50:27 PDT
Created attachment 42172 [details]
KeyboardEvent Test

For reference, I have attached the test I wrote. If I have some time, I'll clean up the test, document it better, and file it under the appropriate bug (or create one if needed).

There are actually three bugs (that I have found so far) that are running around here (ordered by what I think is causing the test to fail): #16735, #9933, #13368
Comment 24 Darin Adler 2009-10-30 10:19:37 PDT
(In reply to comment #22)
> On another note, notice that fast/forms/input-text-enter.html also uses DRT
> eventSender. I suspect this is to work around these bugs.

I don't think that's why. I originally wrote this test three years ago. I think what happened is that I accidentally left both eventSender and DOM keyboard code in the test.
Comment 25 Daniel Bates 2009-10-30 12:43:13 PDT
Do you have any insight into why the test case KeyboardEvent Test does not work?

If you substitute the line initKeyboardEvent line from input-text-enter.html verbatim for the line: keyPressEvent.initKeyboardEvent("keypress", true, true, document.defaultView, "U+000D" /* carriage return */, 0, false, false, false, false, false);

the test still does not run under Safari. The only change I made from input-text-enter.html was that I replaced "Enter" with "U+000D" which is the Unicode value for the carriage return key because "Enter" did not work, and I just wanted to make sure I was simulating the same key code. 

Compare the result of this test under Firefox.

Moreover, notice, the captured keycode for the automatically generated keyboard event is:
(*) keypress - key: U+000D@0 (keyCode/charCode: 0/0) modifiers: false,false,false,false

But it should be something of the form:

keypress - key: U+000D@0 (keyCode/charCode: 13/0) modifiers: false,false,false,false

OR

(**) keypress - key: U+000D@0 (keyCode/charCode: 13/13) modifiers: false,false,false,false

And if you manually submit the test (click to focus on the input field then return the return key on the keyboard), notice  we get something of the form like (**), but the test still fails. The missing keyCode/charCode = 0  in (*) is very troubling (i.e. how does it know what key to press, does it trust the keyIdentifier? or the keyCode? or the charCode?). Hence, this sounds like bug #16735. At least with my test, it doesn't appear to trust the keyIdentifier because it doesn't submit the form, and thus makes me think WebKit is using either the keyCode or charCode when simulating a key but I haven't looked at the code yet to confirm. Also, from writing my test, I was neither able to explicitly set keyCode nor charCode for the event. When I tried, the change was never made. For example, 
add the following code under the line that contains "keyPressEvent.initKeyboardEvent":

keyPressEvent.keyCode = 13;
alert(keyPressEvent.keyCode);

Notice, when the run the test, the alert says "0", which is not 13. Repeat the test substituting charCode for keyCode in the above lines and you will get the same result. Why are these properties act like they are read-only? (I haven't checked the spec yet, but I don't get the feeling that they should be).

(In reply to comment #24)
> (In reply to comment #22)
> > On another note, notice that fast/forms/input-text-enter.html also uses DRT
> > eventSender. I suspect this is to work around these bugs.
> 
> I don't think that's why. I originally wrote this test three years ago. I think
> what happened is that I accidentally left both eventSender and DOM keyboard
> code in the test.
Comment 26 Darin Adler 2009-10-30 12:56:43 PDT
(In reply to comment #25)
> The only change I made from
> input-text-enter.html was that I replaced "Enter" with "U+000D" which is the
> Unicode value for the carriage return key because "Enter" did not work, and I
> just wanted to make sure I was simulating the same key code. 

You can’t just change that. The code uses "Enter" as the key identifier for that key, not "U+000D". This matches the version of the DOM 3 specification at the time we implemented the class.

> Moreover, notice, the captured keycode for the automatically generated keyboard
> event is:
> (*) keypress - key: U+000D@0 (keyCode/charCode: 0/0) modifiers:
> false,false,false,false
> 
> But it should be something of the form:
> 
> keypress - key: U+000D@0 (keyCode/charCode: 13/0) modifiers:
> false,false,false,false
> 
> OR
> 
> (**) keypress - key: U+000D@0 (keyCode/charCode: 13/13) modifiers:
> false,false,false,false

Yes, that's definitely a bug, probably separate from the question of why a DOM-created event doesn't work. To figure out how to make such events work is tricky. The standard ignores the keyCode and charCode properties, so it’s hard to know how they should be set.

> And if you manually submit the test (click to focus on the input field then
> return the return key on the keyboard), notice  we get something of the form
> like (**), but the test still fails. The missing keyCode/charCode = 0  in (*)
> is very troubling (i.e. how does it know what key to press, does it trust the
> keyIdentifier? or the keyCode? or the charCode?).

You are talking here like you can’t read the code that does the form submission. But you can. It’s right there in the WebCore DOM source code. If you find the relevant function you can answer the question about how it figures out which event is a form submission. You should stop guessing about this and look at the source.

> Hence, this sounds like bug
> #16735. At least with my test, it doesn't appear to trust the keyIdentifier
> because it doesn't submit the form, and thus makes me think WebKit is using
> either the keyCode or charCode when simulating a key but I haven't looked at
> the code yet to confirm. Also, from writing my test, I was neither able to
> explicitly set keyCode nor charCode for the event. When I tried, the change was
> never made. For example, 
> add the following code under the line that contains
> "keyPressEvent.initKeyboardEvent":
> 
> keyPressEvent.keyCode = 13;
> alert(keyPressEvent.keyCode);
> 
> Notice, when the run the test, the alert says "0", which is not 13. Repeat the
> test substituting charCode for keyCode in the above lines and you will get the
> same result. Why are these properties act like they are read-only? (I haven't
> checked the spec yet, but I don't get the feeling that they should be).

All tangentially related to why it's hard to use keyboard events to make a test, but possibly not critical. If you ignore some of these side issues you can look at the WebCore DOM source code and figure out what the event would have to have on it to work for testing purposes.

Some of the bugs in the DOM event classes, though, actually reflect DOM 3 standard design issues. It is difficult to use keyboard events that are compatible with the DOM 3 specification, and also the web. It's not clear if DOM 3 offers APIs sufficient to create events that are sufficiently like the real events web pages receive in response to actual input. A closely related issue is how built in behavior in the engine gets information for events. It's actually a kind of deep subject and worthwhile to look at one issue at a time.
Comment 27 Daniel Bates 2009-10-30 13:28:27 PDT
(In reply to comment #26)
> 
> You are talking here like you can’t read the code that does the form
> submission. But you can. It’s right there in the WebCore DOM source code. If
> you find the relevant function you can answer the question about how it figures
> out which event is a form submission. You should stop guessing about this and
> look at the source.

Ok. I'll take a look and get back to you shortly.
Comment 28 Daniel Bates 2009-11-05 18:16:18 PST
Weird, I could have sworn I committed this with bugzilla-tool. Yet, it neither closed this bug (which I need to re-open anyway) nor recorded the changeset of the commit.

Committed r50132: <http://trac.webkit.org/changeset/50132>
Comment 29 Daniel Bates 2009-11-05 18:17:51 PST
Re-opening this bug to work on getting rid of the use of window.eventSender from the layout test committed as part of the patch for this bug.
Comment 30 Daniel Bates 2009-11-05 19:13:23 PST
Comment on attachment 41918 [details]
Patch with test cases

Clearing the commit-queue flag.
Comment 31 Daniel Bates 2009-11-07 19:38:20 PST
On Maciej's suggestion, I am closing this bug since the issue has been addressed. I filed a new bug #31234 to address making the tests cases:
http/tests/misc/isindex-with-no-form-base-href.html
http/tests/misc/isindex-with-no-form.html

work using DOM keyboard events.