Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Web compatibility issue in bricklink.com with "at" method name #41

Open
Constellation opened this issue Jan 21, 2021 · 17 comments
Open

Comments

@Constellation
Copy link
Member

Constellation commented Jan 21, 2021

We enabled at method of Array in ToT WebKit, and encountered web compatibility issue in bricklink.com.
The website includes

            var at = blUtil.getHashVar("at");

            if (at)
            {
                var trigger = JSON.parse(blUtil.decodeURLSafeB64(at));
        getVarsFromHashString:
        function(strHash)
        {
            var vars = [],
                hash;
            var hashes = strHash.slice(strHash.indexOf('#') + 1).split('&');
            for (var i = 0; i < hashes.length; i++)
            {
                hash = hashes[i].split('=');
                vars.push(hash[0]);
                vars[hash[0]] = this.decodeURL(hash[1]);
            }
            return vars;
        }
...
        getHashVars: function()
        {
            return $.getVarsFromHashString(window.location.href);
        }
...
        getHashVar: function(name)
        {
            return $.getHashVars()[name];
        }

It attempted to extract at hash value from window.location.href, and it is using array to construct hash table.
Previously, at returns undefined (if at hash value does not exist) or string (if at hash value exists). But now, it returns function, and getting a wrong code path.

This website does not work with STP 118 and Chrome Canary.

Since at is still common name, I wonder if we need to pick itemAt instead.

@Constellation Constellation changed the title Web compatibility issue with bricklink.com with "at" method name Web compatibility issue in bricklink.com with "at" method name Jan 21, 2021
@Constellation
Copy link
Member Author

ref #34

@ljharb
Copy link
Member

ljharb commented Jan 21, 2021

Is this the only website this occurs in? Is it possible to contact them and get them to update their code?

@MaxGraey
Copy link

Since at is still common name, I wonder if we need to pick itemAt instead.

itemAt already used in pretty popular in 2012 sencha's ExtJS:
https://forum.sencha.com/forum/showthread.php?244131-Extjs4-this-items-itemAt-is-not-a-function

webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Jan 22, 2021
https://bugs.webkit.org/show_bug.cgi?id=220788
rdar://72933608

Reviewed by Saam Barati and Yusuke Suzuki.

Source/JavaScriptCore:

See tc39/proposal-relative-indexing-method#41.

* jsc.cpp:
(CommandLine::parseArguments):
- enable Options::useAtMethod by default for the jsc shell for testing.
* runtime/OptionsList.h:

LayoutTests:

Enable Options::useAtMethod for these tests.

* inspector/model/remote-object-get-properties.html:
* js/Object-getOwnPropertyNames.html:
* js/array-unscopables-properties.html:


Canonical link: https://commits.webkit.org/233253@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271746 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@syg
Copy link
Collaborator

syg commented Jan 25, 2021

Chrome bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1170196

I'll reach out to them and see if they can update their site.

@Constellation Is that code library code or application code? It doesn't read like library code to me, so if they fix it we might be able to stick with at.

Edit: Given that code is on an object name blUtil, which I assume stands for "Bricklink util", I'm going to say this is application code.

@syg
Copy link
Collaborator

syg commented Jan 25, 2021

I've filed a bug on Bricklink via https://www.bricklink.com/helpDesk.asp?helpDeskID=114. I'll keep folks updated on this thread.

yury-s pushed a commit to yury-s/webkit-http that referenced this issue Jan 25, 2021
https://bugs.webkit.org/show_bug.cgi?id=220788
rdar://72933608

Reviewed by Saam Barati and Yusuke Suzuki.

Source/JavaScriptCore:

See tc39/proposal-relative-indexing-method#41.

* jsc.cpp:
(CommandLine::parseArguments):
- enable Options::useAtMethod by default for the jsc shell for testing.
* runtime/OptionsList.h:

LayoutTests:

Enable Options::useAtMethod for these tests.

* inspector/model/remote-object-get-properties.html:
* js/Object-getOwnPropertyNames.html:
* js/array-unscopables-properties.html:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@271746 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@Constellation
Copy link
Member Author

@syg This part is not library/framework code. We are also scheduling evangelist outreach.
As linked to this GitHub issue via GitHub cross-linking, we disabled this for now in WebKit/WebKit@631cd20

@apple-web-evangelist can you share information?

@syg
Copy link
Collaborator

syg commented Feb 9, 2021

Update: I've filed two bugs against bricklink.com and have not yet heard back.

@syg
Copy link
Collaborator

syg commented Mar 2, 2021

Update again: Bricklink's ticket system shows my tickets as "Closed" without any replies. I'll try to escalate, but not sure how yet.

@evilpie
Copy link

evilpie commented Mar 15, 2021

I am going to remove at from Firefox Nightly now.

@syg
Copy link
Collaborator

syg commented Mar 15, 2021

I am going to remove at from Firefox Nightly now.

Hold on, it seems like the repeated tickets did finally get routed to the engineering staff. They replied:

As an update:
we're working on it and will be solved soon.
We have to implement some new features that right now cause some incompatibility.

@evilpie
Copy link

evilpie commented Mar 16, 2021

Nice! I am going to wait if a resolution is imminent.

JonWBedard pushed a commit to WebKit/WebKit that referenced this issue Apr 7, 2021
    Disable Options:useAtMethod because of compatibility issue.
    https://bugs.webkit.org/show_bug.cgi?id=220788
    rdar://72933608

    Reviewed by Saam Barati and Yusuke Suzuki.

    Source/JavaScriptCore:

    See tc39/proposal-relative-indexing-method#41.

    * jsc.cpp:
    (CommandLine::parseArguments):
    - enable Options::useAtMethod by default for the jsc shell for testing.
    * runtime/OptionsList.h:

    LayoutTests:

    Enable Options::useAtMethod for these tests.

    * inspector/model/remote-object-get-properties.html:
    * js/Object-getOwnPropertyNames.html:
    * js/array-unscopables-properties.html:

    Canonical link: https://commits.webkit.org/233253@trunk
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.181@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272940 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@codehag
Copy link

codehag commented Apr 12, 2021

Hey folks, Pinging here to learn if there was any resolution?

JonWBedard pushed a commit to WebKit/WebKit that referenced this issue Apr 12, 2021
    Disable Options:useAtMethod because of compatibility issue.
    https://bugs.webkit.org/show_bug.cgi?id=220788
    rdar://72933608

    Reviewed by Saam Barati and Yusuke Suzuki.

    Source/JavaScriptCore:

    See tc39/proposal-relative-indexing-method#41.

    * jsc.cpp:
    (CommandLine::parseArguments):
    - enable Options::useAtMethod by default for the jsc shell for testing.
    * runtime/OptionsList.h:

    LayoutTests:

    Enable Options::useAtMethod for these tests.

    * inspector/model/remote-object-get-properties.html:
    * js/Object-getOwnPropertyNames.html:
    * js/array-unscopables-properties.html:

    Canonical link: https://commits.webkit.org/233253@main
    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@271746 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Canonical link: https://commits.webkit.org/232923.181@safari-611-branch
git-svn-id: https://svn.webkit.org/repository/webkit/branches/safari-611-branch@272940 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@syg
Copy link
Collaborator

syg commented Apr 13, 2021

I've confirmed with Chrome (when passing --js-flags=--harmony-relative-indexing-methods) that the search function on bricklink.com works again. I will try to ship again soon.

@evilpie
Copy link

evilpie commented May 3, 2021

@syg Have you tried shipping since?

@syg
Copy link
Collaborator

syg commented May 3, 2021

Yes indeed: https://chromium.googlesource.com/v8/v8.git/+/6ac4d69eef83e087abd96f090db6ac74a7fb8038, first stable version will be 92 (if things go well).

@evilpie
Copy link

evilpie commented May 3, 2021

Awesome! Hopefully it will be shipping in Firefox 90 as well: https://bugzilla.mozilla.org/show_bug.cgi?id=1681371

@towerofnix
Copy link

Firefox 90 has shipped, including support for .at()!

Chrome 92 will be shipping just a week later on the 20th, and also supports.at()!

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

No branches or pull requests

7 participants