1545410 - Editing office online word files (Sharepoint farm) in browser does not work

Status: RESOLVED FIXED

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

Description

[baudaxe]

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

Steps to reproduce:

Open any word file (does not matter if uploaded or created new) to edit in browser on a sharepoint farm (actually it's handeled by the Office Online Server, part of the SP farm).

Actual results:

Typing shows up as expected, but if the cursor keys are used to navigate in text or Return is used to insert a line break, all typed text vanishes. Backspace deletes chars just fine from existing text, but one can't add any new chars.

Expected results:

Well, entered text should not be erased by cursor movment or the Return key. It works with different browsers (tested: Chrome, Edge, IE). It isn't a problem of the particular SP farm (SP 2016), it was reproduced in another departement of my company that has its own farm. Firefox works as expected editing Excel sheets. Editing Word files used to work some time ago, unfortunately I don't know which FF update broke it.

Comment 1

[Marcia Knous [:marcia]]

Reporter: Can you please download the latest update, 66.0.2? I think the bug that addressed the issue is in that release.

Comment 2

[baudaxe]

My current stable version is 66.0.3 and I tried the current beta version 67.0b12 too, unfortunately it happens in both versions. If any other steps on my side are necessary to find the issue, please just tell me.

Comment 3

[Marcia Knous [:marcia]]

Hello Dennis - Any idea why this might be happening to this user? This sounds similar to some of the issues we had in Bug 1538966.

Comment 4

[Dennis Schubert [:denschub]]

Interesting. baudaxe, could you do me a favor here? In the Word tab, open the Firefox Developer Tools console (Cmd-Opt-K on macOS, Ctrl-Alt-K on Win/Linux), paste

Array.from(document.querySelectorAll("iframe")).map(f => f.contentWindow)

and copy-paste the output of that command to the bug here? Would really help us out to verify what's going on here!

Comment 5

[baudaxe]

Sure, I modified the output a bit to hide the actual web address and path to the (empty) document I opened to edit:

Array [ Restricted https://somesite.somewhere.nil/we/wordeditorframe.aspx?ui=en%2DUS&rs=en%2DUS&WOPISrc=https%3A%2F%2Fsomesite.somewhere.nil_documentpath%2F%5Fvti%5Fbin%2Fwopi%2Eashx%2Ffiles%2F17cd47e739b748e2b71d991489b297d1&&&wdEnableRoaming=1&wdo=1&wdPid=60B28F7B&wdModeSwitchTime=1556055878352&wdPreviousSession=4fe5d59e-48bd-f044-527c-9735c73ac192&pdcn=pdc71f0 ]

For the record: did this with the stable 66.0.3 version

Comment 6

[Dennis Schubert [:denschub]]

I modified the output a bit to hide the actual web address

Well, that kinda defeated the point a bit, because I wanted to know what domain your Word is running at. :) Is somesite.somewhere.nil your company's domain, or is it a Microsoft-owned one? If you don't feel comfortable sharing details, you can also reach out to me privately via eMail to [e-mail].

Comment 7

[baudaxe]

It is one of our domains, it should be not reachable from outside our firewalls anyway but I still would like to not post the real link here. I will send you mail to the address supplied above with the actual output.

Comment 8

[baudaxe]

Not sure if this helps, but I reactivated an older test system where FF was still at 60.0.2 and gradually updated with versions from ftp.mozilla.org/pub/firefox/releases.
60.0.2 up to 65.0.2 work fine, 66.0 ist the first version that does not work as expected.

Comment 9

[Dennis Schubert [:denschub]]

Thanks for your mail, that was helpful. For the record, we're talking about a custom on-premise installation, and so far, we were not aware that these were affected by those issue as well. Mike, what could we do here? Similar approach than the IBM issue?

baudaxe, unfortunately, there isn't much you can do here right now. If you want to fix the issue for you, you can head over to about:config, and append *.somewhere.nil to the list of domains in the dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode preference. However, since this SharePoint/Word here is on an on-premise install, we can't roll that out to all users, and instead have to figure out another approach. Thanks again so much for the report!

Comment 10

[baudaxe]

Yes, it's a custom on-premise installation of a sharepoint farm, as recommended by Microsoft it consists of 4 servers with seperate roles (SQL-DB, Frontend, Administration/Application, Office Online). Another departement runs a similar farm and faces the same problem.
I noticed you fixed a similar problem with Office365 and Powerpoint and now tried PP on our SP - guess it's no surprise it shows the same problem. We have only few PP presentations in our SP and people usually don't edit in the browser window, that's why nobody noticed.

Thanks for the workaround, but I am afraid I can't motivate ~400 users to apply it :-)

Comment 11

[Mike Taylor [:miketaylr]]

I'm not sure if we can fix this in 66 (unclear if there are any dot releases on the radar), but maybe we can get a fix in for 67.

Comment 12

[Mike Taylor [:miketaylr]]

Masayuki, I guess we need to consider these Microsoft on-premise installs and think of a way to fix this -- we won't be able to whitelist them all. :/

Comment 13

[Mike Taylor [:miketaylr]]

Dennis, do we already know exactly where the offending keypress event handler is? Do we know if a contenteditable element is involved?

Comment 14

[Dennis Schubert [:denschub]]

do we already know exactly where the offending keypress event handler is?

I can't access the installation mentioned here as it is firewalled away, and I don't know an easy way to have the reporter check that. I am assuming they share the code with what they host, in which case https://bugzilla.mozilla.org/show_bug.cgi?id=1536453#c39 would apply. Otherwise, it might be good idea to ask Microsoft if they have a testing installation for us to access.

Do we know if a contenteditable element is involved?

Once again, assuming they share the code, then yes:

<div
  id="WACViewPanel_EditingElement"
  spellcheck="false"
  class="FireFox usehover WACEditing EditMode EditingSurfaceBody"
  style="overflow: visible; visibility: visible;"
  contenteditable="true"
>

Comment 15

[Pascal Chevrel:pascalc]

Tracking for 67 but note that we ship it soon, so any solution or mitigation should be a safe patch to uplift in our last betas next week.

Comment 16

[Pascal Chevrel:pascalc]

Unassigned and our last beta is Friday, wontfix for 67.

Comment 17

[Mike Taylor [:miketaylr]]

Masayuki, we heard back from Microsoft and they said:

We finally determined WACViewPanel_EditingElement is fine to use, so please use that

So if we're able to switch to old legacy behavior for a contenteditable element with an id of WACViewPanel_EditingElement (which is exposed as a global window.WACViewPanel_EditingElement) that should fix on-premise Word Online instances.

They also wrote:

For eventually disabling the fix, could we add an extra class to that same element, something like WACViewPanel_DisableLegacyKeyCodeAndCharCode?

I'm not sure what to think about that -- thoughts?

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Really sorry for the long delay. This was dropped from my queue accidentally.

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

Masayuki, we heard back from Microsoft and they said:

We finally determined WACViewPanel_EditingElement is fine to use, so please use that

So if we're able to switch to old legacy behavior for a contenteditable element with an id of WACViewPanel_EditingElement (which is exposed as a global window.WACViewPanel_EditingElement) that should fix on-premise Word Online instances.

Oh, really great news. I guess that it enables contenteditable editor or designMode editor, if so, we can add hack simply into KeyPressEventModelCheckerChild.

They also wrote:

For eventually disabling the fix, could we add an extra class to that same element, something like WACViewPanel_DisableLegacyKeyCodeAndCharCode?

I'm not sure what to think about that -- thoughts?

Yeah, that's really necessary information since we can stop the hack automatically. Could you verify the exact class name which will be included?

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Reporter: Could you verify one of these builds fixes this bug?

Linux x64: https://queue.taskcluster.net/v1/task/Rv-vvfUtTw23jLl6pQLeCA/runs/0/artifacts/public/build/target.tar.bz2
macOS: https://queue.taskcluster.net/v1/task/dVwM3yWgQoKw8LZ_PavXHQ/runs/0/artifacts/public/build/target.dmg
Windows x64: https://queue.taskcluster.net/v1/task/LvJRO2XAS-GGy9KTmtNdsQ/runs/0/artifacts/public/build/target.zip

Comment 20

[Mike Taylor [:miketaylr]]

Could you verify the exact class name which will be included?

Sure thing, I'll verify that WACViewPanel_EditingElement with a class name of WACViewPanel_DisableLegacyKeyCodeAndCharCode should be the killswitch and update here.

Comment 21

[Mike Taylor [:miketaylr]]

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

Could you verify the exact class name which will be included?

Sure thing, I'll verify that WACViewPanel_EditingElement with a class name of WACViewPanel_DisableLegacyKeyCodeAndCharCode should be the killswitch and update here.

Yep, they've confirmed.

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

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

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

Could you verify the exact class name which will be included?

Sure thing, I'll verify that WACViewPanel_EditingElement with a class name of WACViewPanel_DisableLegacyKeyCodeAndCharCode should be the killswitch and update here.

Yep, they've confirmed.

Thanks, I'll request review for the patches after somebody verifies the build fixes this bug.
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=247725647&revision=275dda402e5576844dc84ffc03d07209b4e0702c

Comment 23

[baudaxe]

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

Reporter: Could you verify one of these builds fixes this bug?

Linux x64: https://queue.taskcluster.net/v1/task/Rv-vvfUtTw23jLl6pQLeCA/runs/0/artifacts/public/build/target.tar.bz2
macOS: https://queue.taskcluster.net/v1/task/dVwM3yWgQoKw8LZ_PavXHQ/runs/0/artifacts/public/build/target.dmg
Windows x64: https://queue.taskcluster.net/v1/task/LvJRO2XAS-GGy9KTmtNdsQ/runs/0/artifacts/public/build/target.zip

Hello, Reporter here. I verified the Windows x64 build fixed the issue, both Word and Powerpoint work again as they should. If necessary I can test the linux build later.

Thanks to everyone involved!

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Thank you very much! I'll request review soon.

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1545410 - part 1: Forcibly disable new keyCode/charCode value of keypress events if the document is Office Online Server

Similar to bug 1514940, we need to use "split model" keypres events on
Office Online Server since they can be installed into user own servers.

Microsoft said it's safe to check whether there is an element whose id is
"WACViewPanel_EditingElement":
https://bugzilla.mozilla.org/show_bug.cgi?id=1545410#c17

Additionally, they'll add new class to the element after fixing the bug in
their side:
https://bugzilla.mozilla.org/show_bug.cgi?id=1545410#c17
https://bugzilla.mozilla.org/show_bug.cgi?id=1545410#c20

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: Bug 1545410 - part 2: Add telemetry probe to decide when we can remove the hack for Office Online Server data-review=liuche,

Comment 27

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: bug1545410.txt

Data review request for attachment 9066965 [details].

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Okay, let's land only the first patch for now (the data-review is not required for it).

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bb0f0291f4c
part 1: Forcibly disable new keyCode/charCode value of keypress events if the document is Office Online Server r=smaug

Comment 30

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/5bb0f0291f4c

Comment 31

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Comment on attachment 9066964 [details]
Bug 1545410 - part 1: Forcibly disable new keyCode/charCode value of keypress events if the document is Office Online Server

Beta/Release Uplift Approval Request

• User impact if declined: Users cannot use Office Online Server which is installed in intranet or something.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• 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): Just adding a hack into the actor which is created for similar bug (bug 1514940). (part 2 is adding telemetry to remove this hack, so, not necessary to uplift and its data-review has not been done yet. So, let's take only the hack for users.)
• String changes made/needed: none

Comment 32

[Julien Cristau [:jcristau]]

Comment on attachment 9066964 [details]
Bug 1545410 - part 1: Forcibly disable new keyCode/charCode value of keypress events if the document is Office Online Server

compat fix for on-prem office online, approved for 68.0b6

Comment 33

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b9d57ba2bf45

Comment 34

[Chenxia Liu [:liuche]]

Comment on attachment 9066967 [details]
bug1545410.txt


1) Is there or will there be **documentation** that describes the schema for the ultimate data set in a public, complete, and accurate way?
Yes, in Scalars.yaml

2) Is there a control mechanism that allows the user to turn the data collection on and off?
Yes, scalars are tied to Firefox data controls

3) If the request is for permanent data collection, is there someone who will monitor the data over time?
No, expires in 71

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?
Type 1, Type 2: whether legacy versions of Office Online Server are being used

5) Is the data collection request for default-on or default-off?
Default on, release

6) Does the instrumentation include the addition of **any *new* identifiers** 
No

7) Is the data collection covered by the existing Firefox privacy notice? 
Yes

8) Does there need to be a check-in in the future to determine whether to renew the data? (Yes/No) (If yes, set a todo reminder or file a bug if appropriate)**
Expiry will kick in

9) Does the data collection use a third-party collection tool? **If yes, escalate to legal.**
No

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ddaa898fff3d
part 2: Add telemetry probe to decide when we can remove the hack for Office Online Server data-review=liuche, r=smaug

Comment 36

[Cristian Brindusan [:cbrindusan]]

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