567213 - selecting editable text and the clicking on an a focusable element does weird things

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Ojan Vafai]

Attachment: keydown testcase

See the test cases. Also, the similar webkit bug: https://bugs.webkit.org/show_bug.cgi?id=38696.

Comment 1

[Ojan Vafai]

Attachment: typing test case

Comment 2

[(no longer active)]

(In reply to comment #1)
> Created an attachment (id=446568) [details]
> typing test case

In this test case, the actual focus is not lost (you can verify this by typing something, and then enter javascript:alert(document.activeElement) in the location bar).  It just happens that the keydown event handler doesn't check if the element is focused, and if typing should actually modify it.

Comment 3

[Ojan Vafai]

(In reply to comment #2)
> (In reply to comment #1)
> > Created an attachment (id=446568) [details] [details]
> > typing test case
> 
> In this test case, the actual focus is not lost (you can verify this by typing
> something, and then enter javascript:alert(document.activeElement) in the
> location bar).  It just happens that the keydown event handler doesn't check if
> the element is focused, and if typing should actually modify it.

I don't follow. Here's the output I see in firebug after each of the three steps in the testcase:
>>> document.activeElement
<p contenteditable="" onkeydown="fail()">
>>> document.activeElement
<button>
>>> document.activeElement
<button>

So, even though typing is going into the contentEditable element, the activeElement is still the button. Note that you need to do this in Firebug in a new window for the test to be valid.

Clicking in the location bar messes up your focus in the document (activeElement returns the body), which itself seems like a bug.

Comment 4

[(no longer active)]

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Created an attachment (id=446568) [details] [details] [details]
> > > typing test case
> > 
> > In this test case, the actual focus is not lost (you can verify this by typing
> > something, and then enter javascript:alert(document.activeElement) in the
> > location bar).  It just happens that the keydown event handler doesn't check if
> > the element is focused, and if typing should actually modify it.
> 
> I don't follow. Here's the output I see in firebug after each of the three
> steps in the testcase:
> >>> document.activeElement
> <p contenteditable="" onkeydown="fail()">
> >>> document.activeElement
> <button>
> >>> document.activeElement
> <button>
> 
> So, even though typing is going into the contentEditable element, the
> activeElement is still the button. Note that you need to do this in Firebug in
> a new window for the test to be valid.

Oh, sure.  That's what I meant.  After clicking on the button, it gets focused.  The problem here is that the keys entered when the button is focused end up modifying the contenteditable section, and the onkeypress event handler has no chance of canceling them because the dispatched event never reaches that element.

> Clicking in the location bar messes up your focus in the document
> (activeElement returns the body), which itself seems like a bug.

I can't reproduce this...

Comment 5

[(no longer active)]

The reason behind this bug is that the HTML editor is attached to the document, and typically installs its event listeners on the body element (if available, otherwise on the documentElement).  Later on, it decides whether something like a keypress should modify the editable areas based on the NODE_IS_EDITABLE flag on document selection's start node (which is set for contenteditbale nodes and all their descendants, as well as the document node for designMode="on" documents), or on the document itself (for the designMode scenario).

This generally means that keypress events do not follow the node chain they're dispatched to, which is the root of the problems in the attached test cases.

Perhaps the least disruptive fix is to check the editable status on the event target and ignore those events targeted to other nodes in the document.

I have a patch which does this.

Comment 6

[(no longer active)]

Attachment: Patch (v1)

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/976a65d34f38