OfficeDev / Office-Addin-Scripts

A set of scripts and packages that are consumed in Office add-ins projects.
MIT License
159 stars 100 forks source link

sideload.js function hasOfficeVersion targetVersion is wrong #467

Closed eblingus closed 3 years ago

eblingus commented 3 years ago

Expected behavior

npm start for Outlook add-in (created using the Yeoman generator for Office Add-ins) will automatically sideload.

Current behavior

sideload fails with message 'The current version of Outlook does not support sideload. Please use version 16.0.13709 or greater.'

In sideload.js, you check the version of Outlook using function hasOfficeVersion, expecting a targetVersion of 16.0.13709 or greater. I think your use of 16.0.13709 as the target version is wrong:
a. Add-ins are supported on Outlook 2013 or later on Windows, and I could not find anything to support your idea that a version of Outlook that supports add-ins does not support sideload. b. My Outlook version is 16.0.13127, which fails your check, but when I removed your check, 'npm start' sideload works just fine. c. Since Outlook 2013+ supports add-ins, and Outlook 2013 is version 15, I suggest your target version be 15.0.0 instead. At version least, it should be no lower than 16.0.0

igor-ribeiiro commented 3 years ago

Hey @eblingus, thanks for the report. That version check was there because sideloading in Outlook wasn't supported in earlier versions. If you try to sideload to Outlook in a previous version, it wasn't expected to actually work.

I will make some tests to make sure the sideloading is working in some earlier versions. We can decrease the number 16.0.13709 if needed. Feel free to also create a PR with some changes if you feel like it.

akrantz commented 3 years ago

@eblingus It would be useful to know more about the Office version you are running. In the File, Office Account page, there is version text next to the About Outlook button. Can you share that with us?

igor-ribeiiro commented 3 years ago

@eblingus I will be closing this issue given no response in the past days. Please feel free to reopen the issue! We always want your help.

eblingus commented 3 years ago

Sorry I missed this earlier. I am currently using 16.0.14131.20278 https://docs.microsoft.com/en-us/officeupdates/update-history-microsoft365-apps-by-date

igor-ribeiiro commented 3 years ago

Sorry I missed this earlier. I am currently using 16.0.14131.20278 https://docs.microsoft.com/en-us/officeupdates/update-history-microsoft365-apps-by-date

Which is a bigger Office version than 16.0.13709. @eblingus I think we can improve the message to talk about Office version and not Outlook versions to avoid this confusion in the future.

akrantz commented 3 years ago

All Office apps have the same version, so what's the confusion between saying Outlook version and Office version?

@igor-ribeiiro It might be that the code which checks the version is not checking each component of the version. Can you double-check this? It needs to check the first number is >= 16, [it can skip checking if second number is >= 0 (of course it is)], and third number is >= 13709.

igor-ribeiiro commented 3 years ago

All Office apps have the same version, so what's the confusion between saying Outlook version and Office version?

@igor-ribeiiro It might be that the code which checks the version is not checking each component of the version. Can you double-check this? It needs to check the first number is >= 16, [it can skip checking if second number is >= 0 (of course it is)], and third number is >= 13709.

I double-checked. The behavior is just like what you said.

igor-ribeiiro commented 3 years ago

As I think this is working as expected, will be closing this for now.