search

OfficeDev / office-js

A repo and NPM package for Office.js, corresponding to a copy of what gets published to the official "evergreen" Office.js CDN, at https://appsforoffice.microsoft.com/lib/1/hosted/office.js.
https://learn.microsoft.com/javascript/api/overview
Other
670 stars 96 forks source link

Files seem to be periodically corrupted by the extensibility framework, as a result Office.addin.setStartupBehavior fails #3993

Open wh1t3cAt1k opened 8 months ago

wh1t3cAt1k commented 8 months ago

Provide required information needed to triage your issue

Office.addin.setStartupBehavior(load) fails on some files which seem to be corrupted.

For more context: our add-in sets the startup behavior to "load" upon initialization. We never expect this operation to fail.

I urge Microsoft to pay special attention to this problem because even if the problem can be worked around, it points to a serious symptom / discrepancy with the way add-ins register themselves in the file.

Your Environment

Steps to reproduce

Consider this file:

before-store-id-fix.xlsx

On this file, when opened in Excel Online:

image

! But it doesn't throw and file is opened fine on Excel Desktop for Mac.

If we unpack the file and go to webextension1.xml:

    <we:reference id="wa200002311" version="1.0.17.11" store="en-US" storeType="OMEX"/>
    <we:alternateReferences>
        <we:reference id="wa200002311" version="1.0.17.11" store="WA200002311" storeType="OMEX"/>
    </we:alternateReferences>

Note that it has a very weird value for "store", seems like the reference ID has overwritten the store attribute! How can this happen? Please do a code review on different platforms (Mac, Windows, Online)!

What I did was to replace store="WA200002311" with store="en-GB" for example, packed it back:

"Fixed" file

after-store-id-fix.xlsx

And on that file, Office.addin.setStartupBehavior(...load) does not throw anymore and file is opened normally with our add-in (Velixo).

Expected behavior

  1. Setting startup behavior does not throw under any circumstances.
  2. The corruption of the file described in this ticket should be automatically fixed on document open.

Context

Some files stopped working and we need to clean them with document inspector, which is Windows-only, and people are complaining of poor UX.

Velixo's external item ID:

Link

xai-msft commented 8 months ago

Hi @wh1t3cAt1k ,

Thanks for contacting us.

Could you let us know how the file 'before-store-id-fix.xlsx' is obtained in the first place? I.e., could you provide some operations that lead to the corruption?

Thanks,

wh1t3cAt1k commented 8 months ago

@xai-msft unfortunately with some bugs we don't have clear reproduction steps how it happened, but clearly the registration of the add-in in the file is out of control of our userland code. It was Excel that corrupted our file.

My best proposal to the MS team is to review, independently in three product teams (Excel for Mac, Online, Excel for Windows) under which cases the ID can be mistakenly written into the store attribute value (IMPORTANT: specifically in the scope of we:alternateReferences).

As I mentioned, below is the erroneous line that causes the symptoms of the bug:

store="WA200002311"

In addition to that, I would recommend Excel Online team to defensively react to such cases and automatically "fix" the store value using the current locale when this bug is encountered.

gmichaud commented 8 months ago

In addition to that, I would recommend Excel Online team to defensively react to such cases and automatically "fix" the store value using the current locale when this bug is encountered.

Important to reiterate that this same file works correctly on Excel desktop -- the logic to identify which add-in to load is more resilient.

xai-msft commented 8 months ago

Hi @wh1t3cAt1k @gmichaud ,

Thanks for your responses. I'd need more information to better locate the issue.

I see that the add-in auto opens in the document that you provided. Just to make sure: have you configured auto-open with any of the methods mentioned here at any point? Or was it achieved solely by calling Office.addin.setStartupBehavior(load)?

In addition, since I have no access to add-in code: does the add-in make other setting related API calls other than setStartupBehavior ? E.g., Office.context.document.settings.set().

wh1t3cAt1k commented 8 months ago

@xai-msft we had auto-open for task pane configured a long time ago, however this file should not be affected by it because we got rid of that logic a while back.

Right now, we simply call Office.addin.setStartupBehavior(load) every time our add-in runs. This is to ensure our add-in's event listeners work whenever the document is opened. We don't want, by default, to show the task pane to the user to avoid any feeling of intrusiveness, as our add-in is custom functions oriented, the user only opens the side panel for specific tasks.

This is a valid usage scenario, according to here, where the option is described as:

image

Which is exactly what we want to achieve.

To answer your other question, yes, we extensively use both Office.context.document.settings for document-scoped report settings, as well as OfficeRuntime.storage for user-scoped add-in settings. The calls to those interfaces are independent of the interaction with the Excel object model, or Excel application state, as I expect the host API to ensure proper synchronization on its side without us having to control this in userland.

I hope this helps the investigation!

xai-msft commented 8 months ago

Thank you for the information! We are currently investigating.

Internal tracking number for the issue that Office.addin.setStartupBehavior(...load) fails on Excel Online: 8682813 Internal tracking number for XML corruption: 8682823

wh1t3cAt1k commented 7 months ago

Thank you @xai-msft, please let us know if you found anything.

wh1t3cAt1k commented 3 months ago

Dear MSFT team, we discovered the issue - I think it's case-sensitiveness of Excel Online to we:reference 's ID element

With the Velixo NX add-in, the below file

1) either fails to load ribbon 2) or gets stuck on loading 3) sometimes results in "Something went wrong" during setStartupBehavior call 4) side panel opens and closes (crash?) during open

2024-06-20-corrupted.xlsx

However, if we change just ONE thing in the unzipped webextension1.xml:

image

(we made the id element uppercase WA from lowercase)

Then things start working. This is the fixed file:

fixed.xlsx

Please see my video below doing this live:

Case-sensitivity to we:reference id element: how it fixes everything in Excel Online - Watch Video

TLDR: Excel Online seems to be unnecessarily case-sensitive to we:reference ID element when recognising add-ins embedded in the document.

IMPORTANT NOTE:

If you create a new Excel file on Windows (!) and save it with a Velixo NX add-in, the "wa" will be lowercase in the XML :) However, I was not able to replicate this on a new and small file, only on this relatively old one that we're using for demos.

So clearly, for cross-platform compatibility, any ID comparisons of the add-ins should be case-insensitive.

What we cannot expect from users is to have to recreate / fix their old files like the one attached - it is too technical for an average user and will produce a frustrating experience.