1099208 - Textbox loses selection and caret when stopping propagation on focus event in capturing event handler on document/window/documentElement

Status: RESOLVED WORKSFORME

(Core :: DOM: Editor, defect)

Description

[Daniel Fisher]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20141113004001

Steps to reproduce:

I focused a text box in ff x dev edition. http://jsfiddle.net/bhkqnv0w/


Actual results:

Nothing.


Expected results:

The caret should be visible and selections in should be be in selected style.

Comment 1

[Daniel Fisher]

Wrong link above, sorry!

And same goes for document and document body

document: http://jsfiddle.net/bhkqnv0w/1/
window: http://jsfiddle.net/bhkqnv0w/2/
document.body: http://jsfiddle.net/bhkqnv0w/1/

Comment 2

[:Gijs (he/him)]

I'm confused. Your testcase calls stopPropagation(), and the handler is attached as a capturing handler. So the focus never arrives in the textbox. Why is this surprising or wrong?

Comment 3

[Daniel Fisher]

I'm confused too :-). FF behaves different than all other browsers...

I tested on Windows: 
- Chrome 38.0.2125.111 m
- IE 11.0.9600.17416
- Opera 25.0.1614.71

Showing the caret has nothing to do with the focus event. 
Seems like FF uses the event to set cursor/caret/selection/range.

Comment 4

[:Gijs (he/him)]

Olli, can you help? :-)

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Interesting. Sounds like an editor issue to me

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Yes, I blame nsEditorEventListener::InstallToEditor().

Comment 7

[:Gijs (he/him)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Yes, I blame nsEditorEventListener::InstallToEditor().

Should the focus and blur events, at least, "just" use system event listeners, and then check getDefaultPrevented?

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Looks like a "recent" regression from bug 283897.

A patch coming.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Attachment: editor_system_events.diff

https://tbpl.mozilla.org/?tree=Try&rev=28b689b31946

This may break quite some tests.
Another option is to not use system group, but current window's parent target and
default group's capture phase.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Actually, might not break anything. But let's wait for the test results.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

(In reply to Olli Pettay [:smaug] from comment #10)
> Actually, might not break anything. But let's wait for the test results.

Hmm, if a focus event handler of web apps sets selection range in <input> or <textarea> wouldn't it break the range at handling focus event in system group? I guess that nsFocusManager or something needs to notify editor of setting focus *before* dispatching DOM focus event.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #11)
> Hmm, if a focus event handler of web apps sets selection range in <input> or
> <textarea> wouldn't it break the range at handling focus event in system
> group?
I don't see what you mean with this.


> I guess that nsFocusManager or something needs to notify editor of
> setting focus *before* dispatching DOM focus event.
This is why I was thinking the other approach to handle these events during capture phase of
default group, but add event listener to nsPIDOMWindow::GetParentTarget

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Attachment: v2

https://tbpl.mozilla.org/?tree=Try&rev=b62cb31dabce

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

https://tbpl.mozilla.org/php/getParsedLog.php?id=56978024&tree=Try&full=1#error0

Masayuki, that show rather ugly bug in IME handling currently. We dispatch a composition event from
focus event listener. We should do it either before or after the focus event.

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Hmm, will that be fixed if we call IMEStateManager::OnFocusInEditor() from ESM::PostHandleEvent() when it receives focus event? If so, I guess it works.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Or, just keep calling it from (new) "focus" event handler but you make new it listen in the system group.

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

The testcase seems to be working now. Please reopen if needed.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sickish, maybe slower respose)]

Perhaps, dup of bug 1450055, but just marking this as depending on it.