Closed Bug 1402376 Opened 2 years ago Closed 2 years ago

Add JAWS client information to update url

Categories

(Toolkit :: Application Update, enhancement)

57 Branch
All
Windows
enhancement
Not set

Tracking

()

VERIFIED FIXED
mozilla58
Tracking Status
firefox56 --- verified
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: jimm, Assigned: rstrong)

References

(Blocks 1 open bug)

Details

(Whiteboard: aes+)

Attachments

(2 files, 2 obsolete files)

We have compatibility issues with 57 w/e10s and the current release of the JAWS accessibility client. A software update from the vendor is forthcoming.

However we can't expect old clients to receive updates and we've exhausted methods for fixing older compat issues. So we want to add the following work around:

1) add the current jaws injection dll version to our updater url
2) use balrog to avoid sending Firefox 57 updates to these users

We'll need to get this change into 56, which is currently at rc2. So this is really pressing.
Assignee: nobody → jmathies
Version: 47 Branch → 57 Branch
Looks like we were using the aus helper for this, in which case we may want to use that again and ship an update in 56. 

http://searchfox.org/mozilla-central/source/browser/extensions/aushelper/bootstrap.js#150
I'm not sure which direction to take here, I could do this in front end js as well, which might be faster. Robert, any suggestions here?
Flags: needinfo?(robert.strong.bugs)
Attached patch aushelper wip v.1 (obsolete) — Splinter Review
ni'ing Ben to get this on his radar.
Flags: needinfo?(bhearsum)
If this can land today, I can still get it into the RC4 build. For now, I'll mark it as a 56 release blocker. 

Or, is this something we can plan to go into a 56 dot release?
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #5)
> If this can land today, I can still get it into the RC4 build. For now, I'll
> mark it as a 56 release blocker. 
> 
> Or, is this something we can plan to go into a 56 dot release?

Either works, whichever causes fewer hassles. Just need to get this to 56 users before 57 ships in November.
Since there will be a requirement to update to 56 let's go with 56. This should add the value to SYSTEM_CAPABILITIES.
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm#87

Specifically, let's go with JAWS: 0 for when JAWS is not detected and JAWS: 1 for when it is detected.
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/UpdateUtils.jsm#138
Flags: needinfo?(robert.strong.bugs)
Meant to say JAWS:0 and JAWS:1 (e.g. no space)
https://bugzilla.mozilla.org/show_bug.cgi?id=1402411 for the Balrog bits needed to support matching on this -- but they don't block the client changes.
Flags: needinfo?(bhearsum)
See Also: → 1402411
Attached patch patch rev1 (obsolete) — Splinter Review
Client code only... still need to check the tests
Assignee: jmathies → robert.strong.bugs
Attachment #8911248 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8911271 - Flags: review?(jmathies)
Attached patch patch rev2Splinter Review
Test changes coming up
Attachment #8911271 - Attachment is obsolete: true
Attachment #8911271 - Flags: review?(jmathies)
Attachment #8911279 - Flags: review?(jmathies)
Attachment #8911279 - Flags: review?(jmathies) → review+
Attachment #8911287 - Flags: review?(jmathies) → review+
I just finished up preliminary testing of this:

STR #1
1) install the 17 or current release version 18 of the JAWS screen reader. 
2) Run JAWS
3) launch Firefox 56 or 57 with this patch and do an update check

AUS:SVC Checker:checkForUpdates - sending request to: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/default/update.xml?force=1

STR #2:
1) install an internal build of the JAWS screen reader (See dbolter, grover, or myself for access)
2) Run JAWS
3) launch Firefox 56 or 57 with this patch and do an update check

AUS:SVC Checker:checkForUpdates - sending request to: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/default/update.xml?force=1

STR #3:
1) do not install jaws
2) launch Firefox 56 or 57 with this patch and do an update check

AUS:SVC Checker:getUpdateURL - update URL: https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-msvc-x64/en-US/default/Windows_NT%206.1.1.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/default/update.xml?force=1
Comment on attachment 8911279 [details] [diff] [review]
patch rev2

Approval Request Comment
[Feature/Bug causing the regression]: not sure... ask jimm
[User impact if declined]: clients with an incompatible jaws client will get updated to a Firefox version that is incompatible with the jaws client
[Is this code covered by automated tests?]: yes. This code is a consumer of nsIXULRuntime shouldBlockIncompatJaws which has been around for awhile and the tests don't cover that code.
[Has the fix been verified in Nightly?]: No. It has been manually verified though.
[Needs manual test from QE? If yes, steps to reproduce]: See comment #14
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is checking the value of nsIXULRuntime shouldBlockIncompatJaws which has been around awhile and based on that adding a string to the update url.
[String changes made/needed]: None
Attachment #8911279 - Flags: approval-mozilla-beta?
Comment on attachment 8911279 [details] [diff] [review]
patch rev2

forgot that this needs to also go on release

Approval Request Comment
[Feature/Bug causing the regression]: not sure... ask jimm
[User impact if declined]: clients with an incompatible jaws client will get updated to a Firefox version that is incompatible with the jaws client
[Is this code covered by automated tests?]: yes. This code is a consumer of nsIXULRuntime shouldBlockIncompatJaws which has been around for awhile and the tests don't cover that code.
[Has the fix been verified in Nightly?]: No. It has been manually verified though.
[Needs manual test from QE? If yes, steps to reproduce]: See comment #14
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It is checking the value of nsIXULRuntime shouldBlockIncompatJaws which has been around awhile and based on that adding a string to the update url.
[String changes made/needed]: None
Attachment #8911279 - Flags: approval-mozilla-release?
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2d9582ceb1f
client code - Add whether the client has an incompatible version of JAWS to the update url. r=jimm
https://hg.mozilla.org/integration/mozilla-inbound/rev/3870c3b2cb54
test code - Add whether the client has an incompatible version of JAWS to the update url. r=jimm
Note, nsIXULRuntime's shouldBlockIncompatJaws landed in 56.
Comment on attachment 8911279 [details] [diff] [review]
patch rev2

JAWS accessibility support, taking it.
Should be in the next 56 rc + 57b3
Attachment #8911279 - Flags: approval-mozilla-release?
Attachment #8911279 - Flags: approval-mozilla-release+
Attachment #8911279 - Flags: approval-mozilla-beta?
Attachment #8911279 - Flags: approval-mozilla-beta+
Henrik, just a heads up. I don't think this will affect your tests.
Flags: needinfo?(hskupin)
https://hg.mozilla.org/mozilla-central/rev/a2d9582ceb1f
https://hg.mozilla.org/mozilla-central/rev/3870c3b2cb54
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
(In reply to Jim Mathies [:jimm] from comment #14)
> STR #1
> 1) install the 17 or current release version 18 of the JAWS screen reader. 
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
> 
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> default/update.xml?force=1

Can this be verified with https://archive.mozilla.org/pub/firefox/candidates/56.0-candidates/build4/win64/en-US/Firefox%20Setup%2056.0.exe ? The dxr results for release look a little sparse - https://dxr.mozilla.org/mozilla-release/search?q=shouldBlockIncompatJaws
Jim, can you verify that shouldBlockIncompatJaws is available per comment #24?
Flags: needinfo?(jmathies)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #22)
> Henrik, just a heads up. I don't think this will affect your tests.

Yeah, shouldn't make a difference. But I don't maintain those tests anymore.
Flags: needinfo?(hskupin)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #26)
> Jim, can you verify that shouldBlockIncompatJaws is available per comment
> #24?

As far as I can tell it was added in bug 1385991, which isn't in 56.
Jim, if the code to support incompatible JAWS didn't make it to 56 could you please get it uplifted? It is very important that this is added to 56 and not a point release so we don't have to create multiple watersheds.

Liz, heads up.
Flags: needinfo?(lhenry)
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #29)
> Jim, if the code to support incompatible JAWS didn't make it to 56 could you
s/incompatible JAWS/incompatible JAWS detection/
Rob, does that prevent us to ship rc4 to the beta population or not?
Any potential bad side effect?
Flags: needinfo?(robert.strong.bugs)
As far as I can tell it'll just never set JAWS:1 in the update URL because Services.appinfo.shouldBlockIncompatJaws is 'undefined'.
I am quite certain comment #32 is the case.

It looks like Jim will need to uplift at least part of bug 1385991 to be able to detect this before updating clients to 57 and it would be a good thing to do this in Firefox 56.0 so we don't have to do one of the following items if the portion of bug 1385991 that is needed is uplifted to a 56.0.x release

1.  releng could create additional updates for pre Firefox 56.0 Windows clients to update to the 56.0.x release with the detection. This is additional work for releng across all locales and additional complexity for the balrog rules.

2. if the additional work by releng is not performed then Windows clients would have to update to 56.0 and then update to the 56.0.x release that contains the detection before updating to latest. Essentially a funnel and funnels take longer by nature which affects orphaning especially for clients that seldom run Firefox.
Flags: needinfo?(robert.strong.bugs)
OK, so, Liz will make the call but I propose that we still push 56rc4 to the beta channel, we build a RC5 and we do extra testing on this bug.
Sounds good?
Agreed, I'd like to push RC4, looking now to see if QE has signed off.
Flags: needinfo?(lhenry)
Jimm, can you request uplift on whatever parts of bug 1385991 and its dependency(s) should get onto 56 before we release 56? Thanks.
I had searched across what I though was 56 and found shouldBlockIncompatJaws last week, but apparently wasn't looking at the 56 code base.
Flags: needinfo?(jmathies)
(In reply to Jim Mathies [:jimm] from comment #14)
> I just finished up preliminary testing of this:
> 
> STR #1
> 1) install the 17 or current release version 18 of the JAWS screen reader. 
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
> 
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> default/update.xml?force=1
> 
> STR #2:
> 1) install an internal build of the JAWS screen reader (See dbolter, grover,
> or myself for access)
> 2) Run JAWS
> 3) launch Firefox 56 or 57 with this patch and do an update check
> 
> AUS:SVC Checker:checkForUpdates - sending request to:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> default/update.xml?force=1
> 
> STR #3:
> 1) do not install jaws
> 2) launch Firefox 56 or 57 with this patch and do an update check
> 
> AUS:SVC Checker:getUpdateURL - update URL:
> https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> msvc-x64/en-US/default/Windows_NT%206.1.1.
> 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> default/update.xml?force=1

Hi Jim, a few questions here:

• The links provided in the URL should be the value of app.update.url in about:config?
• I was asked to test with this treeherder build located here: https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&revision=8c340d0042b510bf984437ca3e5bbbe06daf95d4&selectedJob=133164266 but it seems that the only working build is that of Windows 2012 addon opt. Is that acceptable to test? The Windows 2012 opt is listed as tc(N) and I am unable to open that.
Flags: needinfo?(jmathies)
Flags: needinfo?(jmathies)
(In reply to Grover Wimberly IV [:Grover-QA] from comment #38)
> (In reply to Jim Mathies [:jimm] from comment #14)
> > I just finished up preliminary testing of this:
> > 
> > STR #1
> > 1) install the 17 or current release version 18 of the JAWS screen reader. 
> > 2) Run JAWS
> > 3) launch Firefox 56 or 57 with this patch and do an update check
> > 
> > AUS:SVC Checker:checkForUpdates - sending request to:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:1/default/
> > default/update.xml?force=1
> > 
> > STR #2:
> > 1) install an internal build of the JAWS screen reader (See dbolter, grover,
> > or myself for access)
> > 2) Run JAWS
> > 3) launch Firefox 56 or 57 with this patch and do an update check
> > 
> > AUS:SVC Checker:checkForUpdates - sending request to:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> > default/update.xml?force=1
> > 
> > STR #3:
> > 1) do not install jaws
> > 2) launch Firefox 56 or 57 with this patch and do an update check
> > 
> > AUS:SVC Checker:getUpdateURL - update URL:
> > https://aus5.mozilla.org/update/6/Firefox/58.0a1/20170922132910/WINNT_x86_64-
> > msvc-x64/en-US/default/Windows_NT%206.1.1.
> > 0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:32692,JAWS:0/default/
> > default/update.xml?force=1
> 
> Hi Jim, a few questions here:
> 
> • The links provided in the URL should be the value of app.update.url in
> about:config?
> • I was asked to test with this treeherder build located here:
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-
> release&revision=8c340d0042b510bf984437ca3e5bbbe06daf95d4&selectedJob=1331642
> 66 but it seems that the only working build is that of Windows 2012 addon
> opt. Is that acceptable to test? The Windows 2012 opt is listed as tc(N) and
> I am unable to open that.

Grover and I worked this out over IRC.
Tested the three scenarios with the treeherder 56 build given earlier today, values for JAWS and the URLs match all except the version number from Comment 14 (56.0 as opposed to 58.0a1). Marking as Verified. If there are any further issues, please NI me. Thanks.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.