675579 - The user setting input or textarea direction does not set the element's dir and does not fire the input event

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Aharon (Vladimir) Lanin]

Attachment: setdir.html

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.122 Safari/534.30

Steps to reproduce:

1. Open the attached setdir.html
2. Click on the input and type a.
3. Press Ctrl-Shift-X
4. Click on the input and type b. You will see "rtl." appear next to the period placed above the input earlier.
5. Repeat 1-4 using the textarea instead.

In other words, the user setting an element's direction sets its dir attribute, but does not fire the input event, which is what this bug is all about.



Actual results:

At step 2, a period appeared above the input, indicating that an input event has been fired and that the the element's dir property is null or empty. This shows that the page script is written correctly and is seeing input events.
At step 3, nothing happened, indicating that setting the direction did not generate an input event.
At step 4, another period appeared next to the text placed above the input earlier. This indicates that an input event has been fired, but the element's dir property is  still null or empty.

Thus, setting the input's direction neither sets the element's dir property nor fires an input event. The same happened with the textarea.

This conflicts with the HTML5 spec (http://dev.w3.org/html5/spec/Overview.html#text-state-and-search-state), which states that when the user sets the direction of an input element, the user agent must:

- Set the element's dir attribute to "ltr" if the user selected a left-to-right writing direction, and "rtl" if the user selected a right-to-left writing direction.
- Queue a task to fire a simple event that bubbles named input at the input element.

There is an identical requirement for textarea (http://dev.w3.org/html5/spec/Overview.html#the-textarea-element).


Expected results:

At step 2 in "what did you do?" above, a period should appear above the input, indicating that an input event has been fired and that the the element's dir property is null or empty.
At step 3, "rtl." should have appeared next to the period placed above the input earlier. This indicates that an input event has been fired, and the element's dir property is "rtl".
At step 4, another "rtl." should appear next to the text placed above the input earlier. This indicates that an input event has been fired, and the element's dir property is "rtl".

Comment 1

[(no longer active)]

Fabien, can you work on this, please?  The code responsible for this is nsEditor::SwitchTextDirection.  For firing the input event, you should call FireOnInput on the element's frame, if available (see nsTextInputListener::EditAction for one and currently the only example).

Comment 2

[Mounir Lamouri (:mounir)]

Ehsan, I'm assuming Fabien isn't working on that anymore.

Comment 3

[(no longer active)]

(In reply to comment #2)
> Ehsan, I'm assuming Fabien isn't working on that anymore.

No, I don't think anybody is working on this.

Comment 4

[Aharon (Vladimir) Lanin]

Re-reading my description above, I find myself wondering what I was on when I wrote it. Do follow the steps to reproduce. You will find that when you press Ctr-Shift-X, nothing appears above the input, and when you then add a character to the input, you will only get another dot above the input. Thus, Ctrl-Shift-X neither sets the dir attribute no fires an input event. It is supposed to do both.

Can anyone take on this?

Comment 5

[Aharon (Vladimir) Lanin]

Can someone take on this, please?

BTW, in contentEditable (as opposed to <input> and <textarea>), Ctrl-Shift-X already sets the dir attribute on the contentEditable element, and only the event is missing.

If the event is really bothersome, you could set the dir attribute on the current paragraph inside the contentEditable instead of the contentEditable element itself. That would at least allow Ctrl-Shift-X to set paragraph directionality when editing rich text messages in Gmail. This already works in Chrome, and can be made to work in IE, but there currently no way to do it in Firefox, and people complain about it.

Comment 6

[(no longer active)]

Attachment: Part 1: Set the dir attribute on the input/textarea element when switching the text direction; r=roc

Comment 7

[(no longer active)]

Attachment: Part 2: Dispatch an input event when changing the direction of a plain text element; r=roc

Comment 8

[(no longer active)]

So it turns out that dispatching the input event asynchronously breaks a whole bunch of tests: <https://tbpl.mozilla.org/?tree=Try&rev=d785c74f9a11>.  I could go ahead and fix them all but this makes me wonder if there will be webcompat risks, as input events are very commonly used for things like autocomplete widgets, etc.  So instead of doing that here, I filed bug 960581 to investigate that.

Comment 9

[(no longer active)]

Attachment: Part 2: Dispatch an input event when changing the direction of a plain text element; r=roc

Comment 10

[(no longer active)]

Things looking green: https://tbpl.mozilla.org/?tree=Try&rev=23a286fceea2

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8360729 [details] [diff] [review]
Part 1: Set the dir attribute on the input/textarea element when switching the text direction; r=roc

Review of attachment 8360729 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/libeditor/base/nsEditor.cpp
@@ +5015,5 @@
> +
> +  // For plaintext editors, we need to ask the input/textarea element directly.
> +  if (rootElement->IsRootOfNativeAnonymousSubtree()) {
> +    rootElement = rootElement->GetParent();
> +  }

Let's have a GetExposedRoot method (or pick a better name) that encapsulates this decision.

Comment 12

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e9d82d8ad687
https://hg.mozilla.org/integration/mozilla-inbound/rev/54ddff6ebfa5

Comment 13

[Ed Morley [:emorley]]

Backed out for crashes, eg:
https://tbpl.mozilla.org/php/getParsedLog.php?id=33170493&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=33170882&tree=Mozilla-Inbound

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f5efef8989a5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d8f664832da3

Comment 14

[Ryan VanderMeulen [:RyanVM]]

And reftest failures:
https://tbpl.mozilla.org/php/getParsedLog.php?id=33171735&tree=Mozilla-Inbound

Comment 15

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3475821c2448
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f1ffc0fe914

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3475821c2448
https://hg.mozilla.org/mozilla-central/rev/8f1ffc0fe914

Comment 17

[Aharon (Vladimir) Lanin]

Looking good on Nightly! Thanks for doing this!

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #8)
> So it turns out that dispatching the input event asynchronously breaks a
> whole bunch of tests: <https://tbpl.mozilla.org/?tree=Try&rev=d785c74f9a11>.
> I could go ahead and fix them all but this makes me wonder if there will be
> webcompat risks, as input events are very commonly used for things like
> autocomplete widgets, etc.  So instead of doing that here, I filed bug
> 960581 to investigate that.

I guess you wound up dispatching the event and fixing the tests, right? That is what I am seeing in Nightly, but there has not been any update on your concern re webcomapt risks. IMO, the only risk is that after this change, the value of the input can be the same at two consecutive input events, whereas before this could never be the case. I think it's a pretty small risk - and the fact is that this is the way Blink and WebKit have been behaving for a long time, and it is also in accordance with the HTML5 spec, so I think this is fine.

Re 960581, see my response there.

Comment 18

[Aharon (Vladimir) Lanin]

Oh, is the input event being generated on Ctrl-Shift-X on a contentEditable now too? I hope so.

Comment 19

[(no longer active)]

(In reply to Aharon (Vladimir) Lanin from comment #17)
> Looking good on Nightly! Thanks for doing this!

No worries, sorry I dropped the ball on this!

> I guess you wound up dispatching the event and fixing the tests, right?

I ended up dispatching the event *synchronously*, so I didn't need to fix those tests.

> That
> is what I am seeing in Nightly, but there has not been any update on your
> concern re webcomapt risks.

That risk was about dispatching the event asynchronously.

> IMO, the only risk is that after this change,
> the value of the input can be the same at two consecutive input events,
> whereas before this could never be the case. I think it's a pretty small
> risk - and the fact is that this is the way Blink and WebKit have been
> behaving for a long time, and it is also in accordance with the HTML5 spec,
> so I think this is fine.

Yeah, I'm not much worried about that yet.

Comment 20

[(no longer active)]

(In reply to Aharon (Vladimir) Lanin from comment #18)
> Oh, is the input event being generated on Ctrl-Shift-X on a contentEditable
> now too? I hope so.

Yes it should.  Does it not in your tests?

Comment 21

[Aharon (Vladimir) Lanin]

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #20)
> (In reply to Aharon (Vladimir) Lanin from comment #18)
> > Oh, is the input event being generated on Ctrl-Shift-X on a contentEditable
> > now too? I hope so.
> 
> Yes it should.  Does it not in your tests?

I had not tested it, but I have now, and it is indeed generated now.

However, we have a different problem (unconnected to your changes - FF 26 is the same): Ctrl-Shift-X with the caret in a contenteditable sets the dir attribute *on the body element*! I have filed bug 962015. Please take a look.