Bug #4958

[Workbench] Show an alert message below top nav when running in an unsupported browser.

Added by Ward Vandewege about 5 years ago. Updated about 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Start date:
02/07/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

We've had a user report:

I am now in Firefox and all is working except the upload itself.  I get to
the right page, I can click on browse and select my file, I then click on
upload and get a progress bar but it never fills.  I can click on pause
and it pauses, then on restart and I go back to the progress bar in which
nothing is happening.

He also said that it didn't work for him on Safari (without more detail).

Screenshot from 2015-02-07 17_11_24.png (30.5 KB) Screenshot from 2015-02-07 17_11_24.png Safari 5.1.7 is missing File API Tom Clegg, 02/07/2015 10:13 PM
Screenshot from 2015-02-07 21_31_10.png (58.9 KB) Screenshot from 2015-02-07 21_31_10.png Firefox 3.5.19 is missing File API Tom Clegg, 02/08/2015 02:32 AM
Screenshot from 2015-02-10 17_10_19.png (59.5 KB) Screenshot from 2015-02-10 17_10_19.png Tom Clegg, 02/10/2015 10:12 PM

Subtasks

Task #5171: Check UA string and JS features, throw alert if missing.ResolvedTom Clegg

Task #5174: Add test caseResolvedTom Clegg

Task #5173: Review 4958-old-browser-alertResolvedTom Clegg

Associated revisions

Revision 6bf9ae12
Added by Tom Clegg about 5 years ago

Merge branch '4958-old-browser-alert' closes #4958

History

#1 Updated by Ward Vandewege about 5 years ago

  • Description updated (diff)

#3 Updated by Tom Clegg about 5 years ago

Server logs show the "upload" tab has been tested by some rather old browsers:

Browser Released EOL Prognosis
Safari 5.17 May 9, 2012 2013? 2014? Safari does not have File API until version 7.1. OSX Lion is compatible with <= 6.1.6 and is no longer supported according to Wikipedia
Firefox 12.0 April 24, 2012 June 5, 2012 Firefox does not have File API until version 33. Oldest supported release is 31.3.0 ESR according to Wikipedia

We should have the upload tab check the browser version (and test API support directly with something like if(window.File && window.Blob)), and show a warning message if it doesn't pass muster.

#4 Updated by Tom Clegg about 5 years ago

  • Subject changed from [Workbench] File upload doesn't work on OS X (Lion) in Safari and Firefox to [Workbench] Show an alert message below top nav when running in an unsupported browser.

#5 Updated by Tom Clegg about 5 years ago

  • Target version changed from Bug Triage to 2015-02-18 sprint

#6 Updated by Tom Clegg about 5 years ago

  • Category set to Workbench
  • Assigned To set to Tom Clegg

#9 Updated by Tom Clegg about 5 years ago

  • Status changed from New to In Progress

#10 Updated by Radhika Chippada about 5 years ago

Review comments:

  • The “browser is missing” alert has right margin but no left margin. I think it does not look pretty due to this unbalanced nature. Can you either remove the right border or add same amount of border on left (preferable)?
  • Do we also want to add “WebSocket” to browser_unspported.js? I am aware that we only subscribe to event log messages if this is available and is not a show stopper.
  • If I were to ignore the warning message and continue with login, I continue to see the warning on the top of the page. I think this is ok, but just pointing it out.
    • However, I think in this case one minor suggestion. If we could make the message a bit smaller so that it would fit in one line (in most cases), it could be nicer.
  • BTW, in the alert message do you want to change "you're running an old version..." to "you are running..."? I think it looks too casual this way.
  • Do you want to rename “browser_unsupported.rb” as “browser_unsupported_test.rb” ? All our other tests append _test and this file name threw me off a few times because I was looking for that suffix among all other opened files.
  • Do we want to add tests for Blob and File as well? (If you do this, you might want to rename the existing tests and associated properties to read FileReader instead of File)

#11 Updated by Tom Clegg about 5 years ago

  • The “browser is missing” alert has right margin but no left margin. I think it does not look pretty due to this unbalanced nature. Can you either remove the right border or add same amount of border on left (preferable)?

Eliminated both borders, so it looks more like a stripe than a box. (This also gets rid of the rounded corners, which looked weird next to the top nav.)

  • Do we also want to add “WebSocket” to browser_unspported.js? I am aware that we only subscribe to event log messages if this is available and is not a show stopper.

Good call. Done.

  • If I were to ignore the warning message and continue with login, I continue to see the warning on the top of the page. I think this is ok, but just pointing it out.
    • However, I think in this case one minor suggestion. If we could make the message a bit smaller so that it would fit in one line (in most cases), it could be nicer.

I considered some "hide it after the first page" or "hide it after you click the close button" stuff, but the point of this is to make sure it's impossible to forget (or not know) that you're using Workbench in a way that is known to be broken. I think this goal is incompatible with an "ok, I get it, get that out of my way so I can concentrate" feature.

IOW, improving the experience for people who use Workbench in old browsers is out of scope here. The experience is bad: until/unless we fix that, we should make sure they know why it's bad and how they can fix it.

  • BTW, in the alert message do you want to change "you're running an old version..." to "you are running..."? I think it looks too casual this way.

Done.

  • Do you want to rename “browser_unsupported.rb” as “browser_unsupported_test.rb” ? All our other tests append _test and this file name threw me off a few times because I was looking for that suffix among all other opened files.

Done. I thought the _test was redundant since they're all in the test/ directory (similarly I try to name test functions test 'it works' do... instead of test 'test that it works' do...). But consistency is good, and making it easier to keep track of files seems worthwhile.

  • Do we want to add tests for Blob and File as well? (If you do this, you might want to rename the existing tests and associated properties to read FileReader instead of File)

Not really. :) Killing the FileReader function (part of the HTML5 "File API") is just a way to make the whole "is it supported?" condition fail, so we can verify that failing the condition really causes the alert to appear.

I feel like maintaining two identical lists of conditions would test our ability to keep the lists synchronized, but wouldn't give us any more confidence that our list of conditions is correct. I think the only worthwhile way to test that the list of conditions is working is to use specific combinations of supported/unsupported features that are known to exist in specific versions of real browsers. (And I don't think this branch should have to wait for that.)

Now at 5923d0f

#12 Updated by Radhika Chippada about 5 years ago

LGTM

#13 Updated by Anonymous about 5 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 67 to 100

Applied in changeset arvados|commit:6bf9ae122958b25b4a22447f67fb11cf24765d97.

Also available in: Atom PDF