1538970 - "Enter" key is broken in our IBM Sametime Webchat application when using Firefox 66

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P1)

Description

[[e-mail]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Logged into our IBM Sametime webchat application and clicked on a username to chat. Chat window pops up. Type something in the chat and hit the "Enter" key on the keyboard. It adds a new line but doesn't actually send the message.
This looks like a regression based on these old bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=1497518
https://bugzilla.mozilla.org/show_bug.cgi?id=1497546

We have multiple customers reporting this and is impacting all our users who use webchat.

Actual results:

Unable to send chat message.

Expected results:

The chat message should be sent to the other user.

Comment 1

[Mike Taylor [:miketaylr]]

Is there a URL that we can investigate? Or is this an internally hosted application?

Comment 2

[[e-mail]]

@mike - It's our internal application. But the issue is pretty straightforward as I described above. I can look into providing you access is absolutely needed. We do have a cloud application exhibiting this behavior but you'll need an account to login.

Comment 3

[Mike Taylor [:miketaylr]]

Thanks -- if you could somehow get me access that would be very helpful. Feel free to email [e-mail].

A quick workaround is to add the affected domain to the following pref:

dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode

Comment 4

[[e-mail]]

@mike - sending you an email now with credentials. Please let me know when you are done and I'll go ahead and delete your account.
Thanks!

Comment 5

[Mike Taylor [:miketaylr]]

You can delete the account now, thanks!

I visited https://status.collabserv.com/, and they're aware of the issue and recommend people add the following domains to the blacklist:

-For clients deployed to the AP data center enter: mail.notes.ap.collabserv.com
-For clients deployed to the EU data center enter: mail.notes.ce.collabserv.com
-For clients deployed to the NA data center enter: mail.notes.na.collabserv.com

We know webchat.na.collabserv.com is also affected (from this bug), so we should probably add *.ap.collabserv.com, *.ce.collabserv.com, and *.na.collabserv.com.

Dennis, can you prep a patch?

Comment 6

[Dennis Schubert [:denschub]]

Attachment: Bug 1538970 - Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2c8f1414bd4
Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

Comment 8

[Dennis Schubert [:denschub]]

[Tracking Requested - why for this release]:

This is another patch adding domains to the legacy-charCode pref. Probably too late for 66.0.2, but maybe we can track that for .3, if there will be one?

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f2c8f1414bd4

Comment 10

[Pascal Chevrel:pascalc (PTO until April 26)]

(In reply to Dennis Schubert [:denschub] from comment #8)

[Tracking Requested - why for this release]:

This is another patch adding domains to the legacy-charCode pref. Probably too late for 66.0.2, but maybe we can track that for .3, if there will be one?

Tracking for a potential 66.0.3, thanks.

Comment 11

[[e-mail]]

@Dennis - I see the plan is to add the IBM cloud domains and push it as a potential fix in 66.0.3. While this will resolve our IBM cloud application issues, we have many more onprem customers with different domains who will still see the issue. Is there a plan for a permanent fix?

Comment 12

[Mike Taylor [:miketaylr]]

(In reply to [e-mail] from comment #11)

@Dennis - I see the plan is to add the IBM cloud domains and push it as a potential fix in 66.0.3. While this will resolve our IBM cloud application issues, we have many more onprem customers with different domains who will still see the issue. Is there a plan for a permanent fix?

It's possible we could land a fix similar to what we did for Confluence (Bug 1514940). We would probably need access to an onprem instance (that sounds tricky?) to be able to investigate the feasability though.

In the meantime, those customers can still fix the issue by deploying the pref update:

dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode -> "*.onpreminstancedomain.com"

(if there are existing entries in that pref, they can add the new one after a comma: "foo.com, *.onpreminstancedomain.com").

We should file a new bug for the onprem instances though, since a fix would land in that one.

Comment 13

[Mike Taylor [:miketaylr]]

ni? myself to file a follow up bug.

Comment 14

[[e-mail]]

@Mike - is there no plan to disable dom.keyboardevent.keypress.set_keycode_and_charcode_to_same_value as a permanent fix? Doesn't sound practical to open a ticket with mozilla for every domain.

Comment 15

[Mike Taylor [:miketaylr]]

Right now the plan is to keep the pref, because it fixes many more sites than it breaks. If we're able to come up with a general solution, it would solve it for all the onprem instances, independent of domain. So I will only file a single bug. Thanks.

Comment 16

[Dennis Schubert [:denschub]]

Comment on attachment 9053691 [details]
Bug 1538970 - Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1502795
• User impact if declined: IBM web assests (webmail, webchat, ...) would be broken.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a pref-only change.
• String changes made/needed:

Comment 17

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9053691 [details]
Bug 1538970 - Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

Fix Webcompat issue with IBM webchat application, low risk, uplift approved for 67 beta 6, thanks.

Comment 18

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/15d5b056cd7d

Comment 19

[Dennis Schubert [:denschub]]

Comment on attachment 9053691 [details]
Bug 1538970 - Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1538970
• User impact if declined: IBM web assests (webmail, webchat, ...) would be broken.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a pref-only change.
• String changes made/needed:

Comment 20

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9053691 [details]
Bug 1538970 - Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr

OK to uplift for 66.0.3.
It would be nice to verify this in beta.

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

gaganpreet.saini, can you verify that this is working correctly now for you with Firefox Beta 67? Thanks very much!

Comment 22

[Mike Taylor [:miketaylr]]

(Note, I just emailed Ginni ([e-mail]) about a related IBM matter and got a notification that they're on PTO until April 8)

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-release/rev/7f4781ff8f7d

Comment 24

[Liz Henry (:lizzard) (relman/hg->git project)]

Added to release notes as "FIXED: Keypress events for IBM cloud applications"

Comment 25

[Thomas Wisniewski [:twisniewski]]

Their include.js has this onKeyPress handler (around line 32048):

onKeyPress:function(e){
var c=(e.keyChar&&e.keyChar.toLowerCase())||e.keyCode,_b34=this._keyHandlers[c],args=arguments;
if(_b34&&!e.altKey){
dojo.some(_b34,function(h){
if(!(h.shift^e.shiftKey)&&!(h.ctrl^(e.ctrlKey||e.metaKey))){
if(!h.handler.apply(this,args)){
e.preventDefault();
}

The interesting part is the this._keyHandlers[c]. When things are working, it's looking for _keyHandlers[13], and finds it (it's the method statusMessageContainerKeyPress in the same file). This is what ultimately posts the chat message.

But when things are NOT working, it instead is looking for _keyHandlers["\r"], which doesn't exist, and so statusMessageContainerKeyPress is skipped entirely (and we just get a default line break instead).

The reason for the \r being selected is that Dojo's code sets evt.keyChar like this:

_setKeyChar:function(evt){
evt.keyChar=evt.charCode?String.fromCharCode(evt.charCode):"";
evt.charOrCode=evt.keyChar||evt.keyCode;
}

So when evt.charCode is set to 0 on the Enter keypress event, their code will ultimately do this:

_b34 = this.keyHandlers[e.keyCode === 13] // statusMessageContainerKeyPress

But when the evt.charCode is set to 13, the code will do this:

_b34 = this.keyHandlers[e.keyChar === "\r"] // undefined

Of course this makes sense, since they don't want keypress handling Enter normally, but rather keycode. Unfortunately they're intentionally not listening to keydown Enter in Firefox:

onKeyDown:function(e){
if(!dojo.isMac&&dojo.isSafari&&e.shiftKey&&(e.keyCode==dojo.keys.ENTER)){
this.execCommand("insertHTML"," <br><br>");
}else{
if(dojo.isMozilla&&e.shiftKey&&(e.keyCode==dojo.keys.ENTER)){
}else{
if(e.keyCode==dojo.keys.DELETE||e.keyCode==dojo.keys.BACKSPACE){
this.isDelete=true;
}
}
}

So if they removed that isMozilla check, things should start working.

And here is the isMozilla check in their copy of Dojo:

var dua=n.userAgent,dav=n.appVersion,tv=parseFloat(dav);
d.isKhtml=(dav.indexOf("Konqueror")>=0)?tv:0;
d.isWebKit=parseFloat(dua.split("WebKit/")[1])||undefined;
d.isChrome=parseFloat(dua.split("Chrome/")[1])||undefined;
d.isMac=dav.indexOf("Macintosh")>=0;
var _49=Math.max(dav.indexOf("WebKit"),dav.indexOf("Safari"),0);
if(_49&&!dojo.isChrome){
d.isSafari=parseFloat(dav.split("Version/")[1]);
if(!d.isSafari||parseFloat(dav.substr(_49+7))<=419.3){
d.isSafari=2;
}
}
if(dua.indexOf("Gecko")>=0&&!d.isKhtml&&!d.isWebKit){
d.isMozilla=d.isMoz=tv;
}

This makes me wonder if there is some way we could spoof our navigator.userAgent for legacy sites where we detect a window.dojo instance at the time the UA string is read...

Comment 26

[Thomas Wisniewski [:twisniewski]]

Mike rightly pointed out that my assessment wasn't quite complete, so I took a second shot at this diagnosis to find a fix I could confirm will work.

To revisit the situation, on keypress, Chrome and Firefox get different results for the Dojo-specific keyChar property on the events. This causes IBM's keypress handler (shown in my previous comment) to behave differently:

• in Chrome, e.keyChar ends up being "", so it falls back on e.keyCode, which is 13, which has a handler (which posts the comment).
• in Firefox, e.keyChar ends up being "\r", so it ends up checking for a non-existent "\r" handler (so Firefox ends up not doing anything special and just adds a line-break to the textarea).

This difference comes from the way Dojo's keyChar is computed:

_setKeyChar:function(evt){
evt.keyChar=evt.charCode?String.fromCharCode(evt.charCode):"";
evt.charOrCode=evt.keyChar||evt.keyCode;
}

This looks innocent enough, but in Chrome, e.charCode ends up being 0, while in Firefox it's 13.

It turns out that the 0 in Chrome comes from this Dojo event-fixing helper:

_fixEvent:function(evt,_160){
switch(evt.type){
case "keypress":
if(evt.faux){
return evt;
}
var c=evt.charCode;
c=c>=32?c:0;  // it is happening right here
return del._synthesizeEvent(evt,{charCode:c,faux:true});
}
return evt;
}});

Firefox never runs that code, so the value is never adjusted to 0, and remains 13.

Digging a bit deeper, both browsers reach this code:

_fixCallback:function(name,fp){
return name!="keypress"?fp:function(e){
return fp.call(this,del._fixEvent(e,this));
};

But the value of fp for Firefox is the Dojo base event handler fixer for keypresses:

function(evt,_13b){
switch(evt.type){
case "keypress":
del._setKeyChar(evt);
break;
}
return evt;
}

While Chrome's fp turns out to be overridden with the mixin version above, due to this isWebKit check:

if(dojo.isWebKit){
del._add=del.add;
del._remove=del.remove;
dojo.mixin(del,{add:function(node,_15b,fp){
// snip
_fixEvent:function(evt,_160){ // etc from above

As such, keyChar will not be normalized by Dojo the same way as it does for WebKit.

And since IBM's WebChat relies on keyChar being set to "" instead of "\r", we have this breakage.

Therefore IBM will have to adjust their code to also account for this.

I would strongly recommend that they drop the use of Dojo's keyChar entirely, and just use the now-standard event.key property. That is, change these code-fragments in their include.js:

onKeyPress:function(e){
var c=(e.keyChar&&e.keyChar.toLowerCase())||e.keyCode,_b34=this._keyHandlers[c],args=arguments;

this.addKeyHandler(dojo.keys.ENTER,false,false,dojo.hitch(this,function(){
// snip
}));

To these:

onKeyPress:function(e){
var c=e.key,_b34=this._keyHandlers[c],args=arguments;

this.addKeyHandler("Enter",false,false,dojo.hitch(this,function(){

But that might require extra testing and conversion of other call-points, so a simpler fix would be to just account for "\r" in their keypress handler:

onKeyPress:function(e){
var c=(e.keyChar && e.keyChar!="\r" && e.keyChar.toLowerCase())||e.keyCode,_b34=this._keyHandlers[c],args=arguments;

Alternatively, they could call addKeyHandler twice, once for dojo.keys.ENTER and once for "\r":

var _keyhandler=dojo.hitch(this,function(){
if(dojo.isIE&&!this.isIESend){
// etc
this._renewFormat();
}
});
this.addKeyHandler(dojo.keys.ENTER,false,false,_keyhandler);
this.addKeyHandler("\r",false,false,_keyhandler);

I don't see IBM relying on Dojo's keyChar anywhere else, so these last two hacks seem like safe bets if using event.key isn't as likely.

Of course we still also now have the concern that anything relying on Dojo's keyChar in a similar way is now possibly broken...

Comment 27

[Mike Taylor [:miketaylr]]

I've sent an email to our contact at IBM, thanks Tom.

Comment 28

[adrianomarcondes]

This bug is considerable old. I'm using the most recent version of Firefox and the bug is still present.

Any workaround? Can I tweak my Firefox to make the chat work properly?

Comment 29

[Mike Taylor [:miketaylr]]

(In reply to adrianomarcondes from comment #28)

This bug is considerable old. I'm using the most recent version of Firefox and the bug is still present.

Any workaround? Can I tweak my Firefox to make the chat work properly?

The workaround is to add your domain to dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode.addl from about:config. For example *.yourcoolsite.com.