559561 - When focused element is removed, fire blur event on it

Status: NEW

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Neil Deakin]

Attachment: patch

Currently, the focus is removed from it without firing events. We should be able to fire a blur on it as normal, even when it is no longer in the document.

Comment 1

[Neil Deakin]

OK, the reason this doesn't work for bug 559561 is that scripts are blocked while the ContentRemoved notification occurs. This delays the firing of the blur event until after the content removal has finished when the frame (and the binding?) have already been removed, so the blur listener doesn't get called.

One possibility is to call nsIFocusManager::ContentRemoved at the beginning of nsGenericElement::doRemoveChildAt before the script blocking is in place. The other is to do something like what mutation events do to fire events, but I don't understand what that is.

Or maybe there's some other way?

Comment 2

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

If you want to fire blur while the element is still in document, there isn't
really any other way.
What do other browsers do in this case? Changing how blur works is very regression risky.


(In reply to comment #1)
> OK, the reason this doesn't work for bug 559561
I assume you meant some other bug here, something which deals with XBL bindings
> is that scripts are blocked
> while the ContentRemoved notification occurs. This delays the firing of the
> blur event until after the content removal has finished when the frame (and the
> binding?) have already been removed, so the blur listener doesn't get called.
So where is the blur listener added and how? If the js code was using 
onblur = ... or .addEventListener("blur", ...), it should get the blur event.
If it uses <handler type="blur"> the listener is perhaps removed before the
event is fired.

Comment 3

[Neil Deakin]

(In reply to comment #2)
> If you want to fire blur while the element is still in document, there isn't
> really any other way.

Do you mean that the ways I mentioned are ok?


> What do other browsers do in this case? Changing how blur works is very
> regression risky.

Webkit fires the blur event when content is removed. IE does not.


> > OK, the reason this doesn't work for bug 559561

I meant bug 550366.

<textbox> has a capturing blur listener which resets some focused state. Accessibility also doesn't receive a blur notification.

Comment 4

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

(In reply to comment #3)
> (In reply to comment #2)
> > If you want to fire blur while the element is still in document, there isn't
> > really any other way.
> 
> Do you mean that the ways I mentioned are ok?
If you explain why you need to blur to happen at that time.
And need to be very careful so that there won't be performance regression
nor security problems.

> 
> > What do other browsers do in this case? Changing how blur works is very
> > regression risky.
> 
> Webkit fires the blur event when content is removed. IE does not.
When does Webkit fire the blur event? Before or after removing the
element from document.

Comment 5

[Neil Deakin]

(In reply to comment #4)
> > Webkit fires the blur event when content is removed. IE does not.
> When does Webkit fire the blur event? Before or after removing the
> element from document.

Before removing the element.

Comment 6

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

OK, then it is ok to me to add some code to doRemoveChildAt.
Though the event must not run synchronously when aNotify is PR_FALSE.

And please check if webkit fires blur before DOMNodeRemoved.

If DOMNodeRemoved is fired first in webkit, does the blur fire even if
the node is moved to elsewhere in DOMNodeRemoved event listener?

Comment 7

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

How are you going to handle the case when focused node is a child of the node which will be removed from document? How does webkit handle that?

Comment 8

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

What happens in webkit if focused node is removed from document and in the blur handler the node is appended to be a child of some other child?

Could you test all these kinds of things. And even have mochitests for them when
you write the patch.

Comment 9

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

Webkit seems to have pretty strange bugs related to blur event handling in
this case. If the blur event listener moves the element to somewhere else,
element disappears but later shows up again in the new place.
Apparently this isn't too well tested there.

Comment 10

[Neil Deakin]

It seems that nsINode::ReplaceOrInsertBefore adds a mozAutoDocConditionalContentUpdateBatch before calling RemoveChildAt, so just adding code to doRemoveChildAt isn't going to work.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This is likely implementable by using similar patterns to how we fire DOMNodeRemoved these days. But it's a dirty business. Simply firing blur on the node after it has been removed, and accepting the fact that that means that the event won't reach the Document, is also an alternative.

Comment 13

[Mats Palmgren (inactive)]

Attachment: A test for the display:none case (result is logged in Console)

Comment 14

[Mats Palmgren (inactive)]

Attachment: A test for the node removal case (result is logged in Console)

Comment 15

[Mats Palmgren (inactive)]

The spec also says that we should fire a blur when "it stops being a focusable area"
https://html.spec.whatwg.org/multipage/interaction.html#focus-fixup-rule-one
Which includes styling it with 'display:none':
https://html.spec.whatwg.org/multipage/interaction.html#focusable-area
https://html.spec.whatwg.org/multipage/rendering.html#being-rendered

IE11 and Chrome41 blurs directly on the 'display:none' style change and I think
we should do the same.

Comment 17

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

These days we anyway use script runners to fire focus/blur, so fixing this might not be too difficult.
(display: none case is a separate issue though.)

Comment 18

[Neil Deakin]

Not working on this, but there are lots of ways an element stops becoming focusable that would need to be handled, for example
 - it is hidden
 - it is disabled
 - tabIndex changes
 - the overflow property changes
 - the type on an input element changes
 - etc

So the larger issue is quite involved.

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 21

[Mike Taylor [:miketaylr]]

(ni? myself to talk about in triage)

Comment 23

[Evan Jacobs]

This bug is 10 friggin years old. Can someone fix it?

Comment 24

[reza_karimi76]

And I am here after 11 years! and looking for a fix!!! Wonderful :)

Comment 25

[Sebastian Silbermann]

The spec also says that we should fire a blur when "it stops being a focusable area"

It's not clear that this is implied by

[...] designate the Document's viewport to be the new focused area of the document.

As far as I can tell this does not imply that focus update steps should be run though it would certainly be more useful if document.activeElement is updated and a blur event is fired.

See https://github.com/whatwg/html/issues/1195 and https://github.com/whatwg/html/issues/3162 for similar issues in the living standard, and jsdom/jsdom#2931 for the same issue in jsdom.

Comment 27

[rsnow]

I'm running into this bug trying to implement a Focus Scope component that can contain focus within it. Because blur isn't fired for the activeElement before it's removed and because focus isn't fired for the document body when the focus is inevitably lost there, I have some cases where I cannot contain focus within my component.

Example, a Dialog containing focus within it and the Dialog contents get swapped out due to some action thereby removing the activeElement from the dom.

Comment 28

[Ksenia Berezina [:ksenia]]

Setting webcompat priority as P3 as there are no actual site breakage reports for now.

Comment 29

[rsnow]

I mean, it's breaking us. We've done our best to work around it, but we continue to find edge cases. In addition, it's causing a performance hit and causing us to ship more code.

What we've had to do is:

1. attach a MutationObserver that watches our entire document (childlist and subtree)
2. track focus movement through the entire document as the 'currently focused' element
3. when anything is removed from the document, we check if the 'currently focused' element was within the tree that was removed
4. fire our own blur event on the removed element

Note, this is async, delayed, and doesn't bubble through the tree before removal. So while we track focus movement through the entire document, everything that needs to care if the focused element blurs must attach a blur listener directly to the element. This does not work great when other libraries interact with ours.

Hope this helps anyone else though looking for a workaround.

Comment 30

[Ksenia Berezina [:ksenia]]

Thanks for the feedback :rsnow.

Hi Hsin-Yi, wonder if it's possible to re-prioritize this, based on the comment 29?

Comment 31

[Edgar Chen [:edgar]]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #17)

(display: none case is a separate issue though.)

Yes, the display:none or visibility: hidden case is a different issue which the focus state isn't updated properly at all, we probably could track them separately in bug 1619271. And make this bug focus on missing blur event of element removal.

Comment 32

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Clearing NI - Adam is going to (or already working) on this. Thank you for bringing this again to our attention radar. :)

Comment 33

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 4 duplicates.
:avandolder, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 34

[Adam Vandolder [:avandolder]]

Attachment: Bug 559561 - When focused element is removed, fire blur event on it. r?edgar

Comment 35

[Adam Vandolder [:avandolder]]

Just as an update for anyone who may be following this bug, there is currently a working-ish implementation of behaviour similar to Chrome's implemented in the linked patch. However, due to the above open spec issues, it's unclear what the standard behaviour should be and therefore we're going to have to wait until that is resolved.

Comment 36

[Thomas Wisniewski [:twisniewski]]

Adam, the spec issue here seems to have been resolved recently, and we just found out that GitHub hovercards are similarly affected by focusout not firing when an element is hidden/removed (in https://webcompat.com/issues/117240). Could you advise on whether you feel the resolution here will resolve that as well, or if another spec issue is needed to address focusout separately?

Comment 37

[Adam Vandolder [:avandolder]]

From my understanding of the new focus fixup spec, when an element is removed, blur and focusout events will not be triggered, but instead the document body should be made focused - and that is what this bug will implement.
When an element is hidden, however, blur/focusout should be triggered, and I believe that should be fixed with bug 1810077, although I'm not sure if that addresses the GitHub issue specifically.