Skip to content
This repository has been archived by the owner on Nov 2, 2023. It is now read-only.

Upgrade util.js browser detection #309

Open
florianm opened this issue May 16, 2022 · 3 comments
Open

Upgrade util.js browser detection #309

florianm opened this issue May 16, 2022 · 3 comments
Labels

Comments

@florianm
Copy link
Contributor

florianm commented May 16, 2022

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

@issa-tseng
Copy link
Member

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

@issa-tseng
Copy link
Member

issa-tseng commented May 16, 2022

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.

@florianm
Copy link
Contributor Author

florianm commented May 16, 2022

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants