1540158 - Explore workaround for Bug 1538970, for on-premise IBM webmail instances

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect, P2)

Description

[Mike Taylor [:miketaylr]]

In Bug 1538970, we know of a regression caused by our new keypress behavior.

We're in contact with IBM, and should be able to provide guidance for them to fix the cloud-hosted webmail, as well as their on-premise version.

However, existing on-premise installs that will need some kind of workaround.

I'm filing this bug to explore if we can do something similar like Bug 1514940.

Ideally we could also add telemetry or something to know when we can remove it in the future.

Comment 1

[Mike Taylor [:miketaylr]]

Update: our contact with IBM said they will (re-)get me access to the webmail on Monday. Will post details of the issue when I have them figured out.

Comment 2

[sisingh]

Hi Mike,

Could you help share the personnel you are in contact with, I can reach out to him personally to expedite access(if that hasnt been done yet)
I created the original bug https://bugzilla.mozilla.org/show_bug.cgi?id=1538317, and following up on the final resolution via this ticket.
Regards,
Siddhartha

Comment 3

[Mike Taylor [:miketaylr]]

Hi Siddhartha,

gaganpreet.saini@ has been helping me. It seems the test account only has access to chat, but I've emailed and asked for webmail support to be added to the [e-mail] account. Thanks.

Comment 4

[sisingh]

Thanks for your response Mike.
In case you would like to, you can go ahead and Register a trial account here:

https://apps.na.collabserv.com/manage/account/public/trial/showCreateTrialAccount?campaignCode=BP1838943680&bp=1

If this works for you !

Regards,
Siddhartha

Comment 5

[Thomas Wisniewski [:twisniewski]]

Mike and I just did a diagnosis of what's going here, and it's... fun.

It turns out that they are preventing the default action on p keypress events here in the markup:

AAA.ESb.onkeypress=function(ev){if (ev.keyCode != ev.DOM_VK_F1) return;AAA.DSq.ELU(null, 'onHelpHandler', 'FCe' ,{sAction: "onhelp"});ev.preventDefault();}

That happens because ev.keyCode == DOM_VK_F1 == 112, so their code does not do the early return, and ultimately thinks it's an F1 keypress to be defaultPrevented.

It worked before because our old code defined keyCode == 0, so their code did not believe it was an F1 keypress (it might be worth noting that in both cases charCode == 112).

And it works in Chrome because they don't specify a DOM_VK_F1, it's undefined, and since 112 != undefined, the early return kicks in.

In fact, I don't think this code is doing anything in any browser anymore, if it ever did. The keypress handler is never hit for F1 keys on most browsers, and to my knowledge only Gecko ever defined DOM_VK_F1 (and I don't think it ever set keyCode to a non-0 value).

So perhaps this is actually a long-broken feature? (They would likely want to test ev.key == "F1" these days, and in a keydown handler instead of keypress).

I also wonder if it's a good idea for us to try to undefine those DOM_VK_x constants now? (or maybe only keep them for system/addon pages?)

Comment 6

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

NI Masayuki for comment 5.

Comment 7

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

And it works in Chrome because they don't specify a DOM_VK_F1, it's undefined, and since 112 != undefined, the early return kicks in.

Ahhhh, really tricky!!

So perhaps this is actually a long-broken feature? (They would likely want to test ev.key == "F1" these days,

Yes, it is. Or, just drop it since no browsers dispatch F1 keypress event anymore if they don't support Firefox ESR 60.

and in a keydown handler instead of keypress).

Oh, then, why didn't they handle the keydown for F1 already on Firefox? Is it ignored only on Firefox??

I also wonder if it's a good idea for us to try to undefine those DOM_VK_x constants now? (or maybe only keep them for system/addon pages?)

Hmm, I guess that some web apps may check it for feature detection whether it's Gecko or not.

Smaug, can we collect usage of consts of WebIDLs with telemetry or something?

Comment 8

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

Let me test...

Comment 9

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

Looks like not easily. The currently codegenerator gives
WebIDL.WebIDLError: error: Unknown extended attribute UseCounter on constant.

bz, would there be any reasonable way to check the usage of webidl constants?

Comment 10

[Thomas Wisniewski [:twisniewski]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Might be slow response) from comment #7)

Oh, then, why didn't they handle the keydown for F1 already on Firefox? Is it ignored only on Firefox??

I don't think they have any other F1-handling code, as Chrome just opens its own F1 info page in a new tab, and nothing happens in WebMail. I do think they could probably just move their code to keydown and check for evt.key==="F1", if they want (that works for me in Chrome and Firefox, at least). Perhaps this was actually a Firefox-specific info page, or this was written before Chrome and other browsers came out?

Comment 11

[Boris Zbarsky [:bzbarsky]]

The problem with consts is that they are just data properties on the object. We define them, and that's it; the JS engine can just get the value without interacting with any of the binding machinery.

I can think of two ways to intercept these property gets:

1. Change them from consts to readonly attributes (regular and static, so they show up on the proto and the interface object). This is observably different from the current behavior, both via getOwnPropertyDescriptor and via being able to redefine them, though maybe we could hardcode them to be non-configurable. This might be OK for nightly and early beta, but I would not want to do it on late beta or release.

2. Change the constructor and proto to be proxies, which would allow us to intercept the gets. This would require a whole bunch of surgery on bindings code, unfortunately.

Comment 12

[Mike Taylor [:miketaylr]]

So how terrible is it for this bug to detect the environment, and just set DOM_VK_F1 to undefined for just this page (iNotes)?

(assuming this magic JS would actually do what I want it)

if (window.com_ibm_nr_info) {
 KeyEvent.DOM_VK_F1 = undefined;
}

That would handle legacy IBM Notes, and we could ask IBM to fix upstream. It would be nice to know if there isn't a more reliable object we could hook into.

Disabling it wholesale sounds scary to me.

Comment 13

[Thomas Wisniewski [:twisniewski]]

Unfortunately, KeyEvent.DOM_VK_F1 isn't a configurable JS property, so we would need to somehow allow it to be configurable, or find another way to configure it outside of the regular JS methods.

Comment 14

[Mike Taylor [:miketaylr]]

(yeah my sample "magic" code would need to take care of that in C++, probs)

Comment 15

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

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #11)

1. Change them from consts to readonly attributes (regular and static, so they show up on the proto and the interface object). This is observably different from the current behavior, both via getOwnPropertyDescriptor and via being able to redefine them, though maybe we could hardcode them to be non-configurable. This might be OK for nightly and early beta, but I would not want to do it on late beta or release.

Hmm, I think that such usage data isn't useful unless we'd get too big number since the volume of our testers is not so big. (E.g., nobody tested Microsoft Office 365 before release.)

2. Change the constructor and proto to be proxies, which would allow us to intercept the gets. This would require a whole bunch of surgery on bindings code, unfortunately.

Well, yeah, of course, we couldn't take this approach...

Comment 16

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

(In reply to Mike Taylor [:miketaylr] from comment #12)

So how terrible is it for this bug to detect the environment, and just set DOM_VK_F1 to undefined for just this page (iNotes)?

(assuming this magic JS would actually do what I want it)

if (window.com_ibm_nr_info) {
 KeyEvent.DOM_VK_F1 = undefined;
}

That would handle legacy IBM Notes, and we could ask IBM to fix upstream. It would be nice to know if there isn't a more reliable object we could hook into.

Disabling it wholesale sounds scary to me.(In reply to Thomas Wisniewski [:twisniewski] from comment #13)

Unfortunately, KeyEvent.DOM_VK_F1 isn't a configurable JS property, so we would need to somehow allow it to be configurable, or find another way to configure it outside of the regular JS methods.

I think that we should disable the new behavior if we can detect IBM webmail which does not have fix. If there is a JS object which includes version number and IBM webmail has a contenteditable, we can insert new check into KeyPressEventModelCheckerChild (I mean we would fix it easily).

Comment 17

[Boris Zbarsky [:bzbarsky]]

If we have a way to detect the webmail pages from C++, not exposing DOM_VK_F1 on it is also pretty simple with a [Func] annotation on the IDL constant... Ugly, but doable. The question is how long we'd need to keep that fix.

Comment 18

[Mike Taylor [:miketaylr]]

(I've emailed IBM and asked if they have recommendations for detecting IBM webmail with a higher degree of confidence than my guess)

Comment 19

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating webcompat priority whiteboard tags to project flags.

Comment 20

[Thomas Wisniewski [:twisniewski]]

(removing a double comment)

Comment 21

[Thomas Wisniewski [:twisniewski]]

Note that if we're comfortable with only targeting known WebMail instances, we should be able to use our webcompat system addon to proxy over the KeyEvent interface itself, and have the proxy return undefined for DOM_VK_F1. This would be a simpler fix, but of course we would need to know the affected domains to keep the solution efficient (since it involves running a content script to do the proxying).

Also, for the record there are no web platform tests for these DOM level 3 constants. And since this seems to be a bug with IBM's webmail, rather than a widespread compat issue, I think it might be worthwhile to add WPTs ensuring those constants are defined for KeyEvent. However, Firefox defines those constants on KeyboardEvent as well, which I believe is incorrect (according to the working draft).

Comment 22

[Henri Sivonen (:hsivonen)]

twisniewski, with comment 17 and comment 21, is there enough information to proceed to writing a patch? Is this P1 something you could take?

Comment 23

[Thomas Wisniewski [:twisniewski]]

Yes and no. I can (and will) write a test-patch, but we still need to figure out a way to detect affected Webmail instances (based on URL? or are there still-affected domains we can at least apply the fix for?)

Comment 24

[Mike Taylor [:miketaylr]]

(I reached out to our IBM contacts to get us test accounts again so we can pick this up)

Comment 25

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

Moving to P2 as without more information from IBM, hard for us to make more progress here.

Comment 26

[chaitanyagrampurohit]

Has this bug been resolved or any other workaround available? I tried the workaround at https://support.mozilla.org/en-US/questions/1253981 but that is not working.

Comment 27

[Mike Taylor [:miketaylr]]

Which part of the workaround did you try? And which version of Firefox are you on? Feel free to email me at [e-mail] with more details, thanks.

Comment 28

[chaitanyagrampurohit]

I added value-'ongc.co.in' in exceptions in the 'dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode' in about:config settings(as explained in the https://support.mozilla.org/en-US/questions/1253981)
This is not working. Whenever 'p' character is pressed in address section of Lotus Mail, the Help section pops out.
Firefox version is 70.0.1 (64-bit).

Comment 29

[Mike Taylor [:miketaylr]]

(following up via email)

Comment 30

[Mike Taylor [:miketaylr]]

ni? myself to see if we can get IBM to give us an account (again....)

Comment 31

[Mike Taylor [:miketaylr]]

I sent another email.

Comment 32

[Mike Kaply [:mkaply]]

I think we can close this, can't we? IBM did the fix and there was nothing we could do?

Comment 33

[Mike Taylor [:miketaylr]]

Probably, though it makes me a bit uneasy.