903815 - Escape key doesn't close the Facebook chat window

Status: RESOLVED FIXED

(Tech Evangelism Graveyard :: English US, defect)

Description

[kurtbellamy7]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20130728 Firefox/25.0 (Nightly/Aurora)
Build ID: 20130728030204

Steps to reproduce:

Open facebook and have a facebook chat window open, and as of Mozilla/5.0 (Windows NT 6.2; WOW64; rv:25.0) Gecko/20130728 Firefox/25.0 ID:20130728030204 CSet: 73b69c146ca6
the escape key no longer closes the individual windows. 


Actual results:

when the escape key is pressed, with a chat window highlighted, it densest close


Expected results:

The chat windows used to close when they escape key was pressed.

Comment 1

[Loic]

Are you speaking about the social service Facebook Messenger for Firefox or the chat available on every Facebook account?

Comment 2

[kurtbellamy7]

sorry, its on the web based chat at www.facebook.com

Comment 3

[Loic]

Thanks for the report, confirmed. Maybe a TE bug due to new feature in Firefox.

Regression range:
good=2013-07-25
bad=2013-07-26
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a4c1961bf723&tochange=46d73e889cb4

Suspected bug:
Masayuki Nakano — Bug 501496 part.12 tilt-visualizer should use keydown event handler instead of keypress event handler for Escape key r=vp
Masayuki Nakano — Bug 501496 part.11 <browser> should not handle keydown event when escape key is pressed during auto scroll r=enndeakin

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Loic from comment #3)
> Thanks for the report, confirmed. Maybe a TE bug due to new feature in
> Firefox.

I think so. Probably, keydown event handler for <textarea> or its ancestor calls preventDefault(). And keydown event for Escape key is handled in keypress event. Additionally, such path is run only on Firefox since other browsers don't work with such code.

Unfortunately, it's too difficult to check the cause with our side because facebook's script is too complex. I don't find the code handling ESC key. Should I contact facebook personally? Or can we contact facebook officially? If the latter is available, I recommend it because unofficial contact might be ignored as I experienced with some web services.

Comment 5

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

CCing some FB folks (at least I think they are still @FB).

Comment 6

[Loic]

Issue only in FF24+.

Comment 7

[blizzard]

I'm debugging in Firefox 23 and it's definitely reproducible here.

Anyway, just debugged the JS code.  The input handler for the text area never gets the keydown event.  As Masayuki points out, that might be something else eating the event before it gets there.  I'll have to debug more.

Comment 8

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

FF23 issue is certainly something else. Bug 501496 landed to FF25.

Comment 9

[blizzard]

OK, fair enough.

Comment 10

[blizzard]

The reason why Firefox gets a different code path than every other browser is because of the key handling for Korean doesn't fire a keydown event during composition so we listen to keypress instead:

https://bugzilla.mozilla.org/show_bug.cgi?id=354358

Note that if I change the UA checker to use the same event path on Chrome it shows the exact same issue.  If I use keydown instead of keypress on the ESC key handling path Firefox doesn't have the problem.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

While there is composition of IME, Gecko never fires any key events between compositionstart event and compositionend event. So, I don't understand your comment well. Why does listening keypress help you?

And it sounds like FB uses keypress event handler for handling both non-printable key events like Escape key and text input. If so, IIRC, it's the best approach to use keydown event for the former and input event for the latter on all browsers.

Comment 12

[blizzard]

The key handler is used for hot keys not to reproduce key input.  Return and ESC in particular.  (I don't think we listen for composition events at all.)

Comment 13

[blizzard]

Just to be clear, we might have buggy code.  I'm trying to see if I can find someone to reproduce the issue with Korean input and see if it's required.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to the comment 7, this is not a new regression by our change. Anyways, comment 13 suspects that this is a bug of facebook.

Shouldn't we move this to evangelist bug?

Comment 15

[Alex Keybl [:akeybl]]

Any updates? Are we decided on this being a TE issue?

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I guess so.

Comment 17

[blizzard]

Tried it in Aurora (Firefox 26.0a2)  Still broken there.

Works fine in Firefox 24.

Comment 18

[kurtbellamy7]

bug fixed as of nightly 27.0a1 (2013-10-23)

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Can someone verify that this is no longer broken on 26?

Comment 20

[Loic]

It's fixed in Firefox 25 (and later), pressing "Esc" closes the Facebook chat window.
I guess Facebook fixed the issue on its side.

Comment 21

[blizzard]

Yes, we did.

Comment 22

[juan becerra [:juanb]]

This works for me as well. Tested on Fx26b2. I can dismiss chat windows with the escape key.

Comment 23

[Josh Matthews [:jdm]]

WFM is more appropriate when we made no changes.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Josh Matthews [:jdm] from comment #23)
> WFM is more appropriate when we made no changes.

Why? This bug is an evangelist bug and Facebook actually fixed this bug on their side (comment 21).

Comment 25

[Kevin Brosnan]

Masayuki is correct this is an Evang issue.