build icon indicating copy to clipboard operation
build copied to clipboard

Upgrade util.js browser detection

Open florianm opened this issue 3 years ago • 3 comments

Suggestion 1: switch Firefox detection

Shown in production on Firefox 100.0 (64-bit):

InstallTrigger is deprecated and will be removed in the future.

The warning I'm encountering in the year 2022 is caused by this bit written 5 years ago against a quite younger Firefox:

// browser detection, because standards are apparently for suckers. based on:
    // http://stackoverflow.com/questions/9847580/how-to-detect-safari-chrome-ie-firefox-and-opera-browser/9851769#9851769
    // ugh; see the commit message @c1c897e for more details.
    $.isChrome = Boolean(window.chrome) && Boolean(window.chrome.webstore);

    // and these are necessary because Firefox and Safari alone do not auto-scroll
    // near margins when dragging whilst other browsers do, and neither behaviour is
    // easily detectable without causing some artifacts.
    $.isFirefox = ((typeof InstallTrigger) !== 'undefined');
    //$.isFirefox = Boolean(window.netscape) && / rv:/i.test(navigator.userAgent); // keeping this alternative in case the above stops working.

For now, InstallTrigger still exists, and the above check still works. The alternative check (commented out in snippet above) also works - it returns true in Firefox and false in Chrome.

Suggestion 2: Change Chrome detection

Using Chrome 101.0.4951.64 (Official Build) (64-bit) in Ubuntu 22.04 in May 2022,

    // browser detection, because standards are apparently for suckers. based on:
    // http://stackoverflow.com/questions/9847580/how-to-detect-safari-chrome-ie-firefox-and-opera-browser/9851769#9851769
    // ugh; see the commit message @c1c897e for more details.
    $.isChrome = Boolean(window.chrome) && Boolean(window.chrome.webstore);

incorrectly returns false for Chrome because Boolean(window.chrome.webstore) is false. $.isChrome = Boolean(window.chrome) works but I haven't investigated possible side effects with other browsers.

Suggestion 3: Windows and Mac OS detection might be deprecated

VSCode warns on

$.isWindows = /win/i.test(navigator.platform);
    $.isMac = /mac/i.test(navigator.platform);

about

(property) NavigatorID.platform: string
@deprecated

'platform' is deprecated.ts(6385)
lib.dom.d.ts(9724, 9): The declaration was marked as deprecated here.
No quick fixes available

Further resources: MDN on browser detection and why user agent sniffing (which Build is not doing) is bad: https://developer.mozilla.org/en-US/docs/Web/HTTP/Browser_detection_using_the_user_agent

florianm avatar May 16 '22 02:05 florianm

user agent sniffing is bad.. browser specific bugs are worse.

issa-tseng avatar May 16 '22 15:05 issa-tseng

if you can make this code work without browser detection (cross window dragging between Build tabs, plus holding alt/option to clone rather than move) you are 100x the programmer i am.

issa-tseng avatar May 16 '22 15:05 issa-tseng

Probably best to keep as unchanged as possible.

The comments in draggable.js had me in tears - that file is 🛑🔨🕑 because ...I can't touch this.

Would it be safe to patch the FF and Chrome detection as outlined?

florianm avatar May 16 '22 23:05 florianm