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

RESOLVED FIXED in Firefox 66

Status

()

defect
P1
normal
RESOLVED FIXED
a month ago
2 days ago

People

(Reporter: gaganpreet.saini, Assigned: denschub, NeedInfo)

Tracking

(Regression, {regression})

66 Branch
mozilla68
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(relnote-firefox 66+, firefox-esr60 unaffected, firefox66+ fixed, firefox67+ fixed, firefox68+ fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

a month ago

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.

(Reporter)

Updated

a month ago
Summary: "Enter" key is broken in our IBM Sametime Webchat application → "Enter" key is broken in our IBM Sametime Webchat application when using Firefox 66

Updated

29 days ago
Component: Untriaged → DOM: Events
Product: Firefox → Core
Depends on: 1538966

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

Flags: needinfo?(gaganpreet.saini)
(Reporter)

Comment 2

29 days ago

@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.

Flags: needinfo?(gaganpreet.saini)

Thanks -- if you could somehow get me access that would be very helpful. Feel free to email miket@mozilla.com.

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

dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode

(Reporter)

Comment 4

29 days ago

@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!

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?

Flags: needinfo?(dschubert)
Assignee: nobody → dschubert
Priority: -- → P1
(Assignee)

Updated

29 days ago
Flags: needinfo?(dschubert)

Comment 7

29 days ago
Pushed by mitaylor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f2c8f1414bd4
Add IBM domains as wildcard exceptions for legacy keyCode and charCode. r=miketaylr
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 8

29 days ago

[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

28 days ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 28 days ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
(Assignee)

Updated

28 days ago
No longer depends on: 1538966

(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.

(Reporter)

Comment 11

28 days ago

@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?

(In reply to gaganpreet.saini@hcl.com 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.

ni? myself to file a follow up bug.

Flags: needinfo?(miket)
(Reporter)

Comment 14

28 days ago

@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.

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.

(Assignee)

Comment 16

28 days ago

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:
Attachment #9053691 - Flags: approval-mozilla-beta?

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.

Attachment #9053691 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(Assignee)

Comment 19

27 days ago

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:
Attachment #9053691 - Flags: approval-mozilla-release?
Flags: qe-verify-
See Also: → 1540158
Flags: needinfo?(miket)

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.

Attachment #9053691 - Flags: approval-mozilla-release? → approval-mozilla-release+

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

Flags: needinfo?(gaganpreet.saini)

(Note, I just emailed Ginni (gaganpreet.saini@hcl.com) about a related IBM matter and got a notification that they're on PTO until April 8)

No longer blocks: 1502795
Regressed by: 1502795

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

Comment 25

14 days ago
partly-obsolete

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...

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...

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

You need to log in before you can comment on or make changes to this bug.