Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LibWeb: Product images don't load on twinings.co.uk #21009

Closed
awesomekling opened this issue Sep 10, 2023 · 4 comments · Fixed by #21700
Closed

LibWeb: Product images don't load on twinings.co.uk #21009

awesomekling opened this issue Sep 10, 2023 · 4 comments · Fixed by #21700
Labels
bug Something isn't working fund This issue can be funded via polar.sh web compatibility

Comments

@awesomekling
Copy link
Contributor

awesomekling commented Sep 10, 2023 •
edited by polar-sh bot
Loading

At some point recently, https://twinings.co.uk switched from using OneTrust for cookie consent to using Consentmo.

The new system doesn't load or show its banners for some reason. This prevents the site from working correctly as asset loading is inhibited until after cookies are consented to.

To reproduce (from Europe), simply go to https://twinings.co.uk and notice that you're not being hassled about cookies.

cc @Lubrsi

Funding

  • We're currently trying Polar.sh where you can help fund the work on this issue.
  • Contributor(s) receive the funding once the issue is completed & confirmed.
Fund with Polar
@awesomekling awesomekling added the bug Something isn't working label Sep 10, 2023
@ADKaster ADKaster moved this to Todo in Ladybird Sep 10, 2023
@awesomekling awesomekling added web compatibility fund This issue can be funded via polar.sh labels Oct 24, 2023
@awesomekling awesomekling changed the title LibWeb: Consentmo cookie consent system doesn't load on twinings.co.uk LibWeb: Product images don't load on twinings.co.uk Oct 26, 2023
@awesomekling
Copy link
Contributor Author

As of today, there is no cookie banner in Firefox or Chrome either. However, we still don't show product images, so something is still going wrong :)

@TobyAsE
Copy link
Member

TobyAsE commented Oct 28, 2023

Observation: This error only occurs on the live page, not on a dumped version.

@krkk
Copy link
Contributor

krkk commented Oct 28, 2023 •
edited
Loading

I've took a look at this issue and it seems the page never completes loading: document.readyState always spits out ‘interactive’ instead of ‘complete’, the lazyload.87302ac671b53831.js script is never requested, and the entire webpack loader script (main.js) never executes.

I suspect that all this caused by this little inline module script placed slightly before the main script:<script type="module">!function(o) {(o.Shopify=o.Shopify||{}).modules=!0}(window);</script>
The problem is not with the contents of the script, but rather that it‘s a module, and the script is inlined.

You see, we first mark the script as ready (32.2.2.2.1) and later we set that the script as ready to be parser-execute, when the script is marked ready (33.4.2), effectively blocking the parsing completion.1

it is unlikely to be a spec bug. Our implementation of fetching the descendants of and linking a module script is outdated, doesn't use promises, and I suspect we would need to implement ecma2023's HostLoadImportedModule hook changes, as well as other things related to modules in LibJS in order to catch up.

Footnotes

  1. small repro: 

    <!doctype html>
    <script type=module>console.log('never reached :(')</script>
    ↩

@networkException
Copy link
Member

Hey, sorry for not commenting earlier.

I came to the same conclusion debugging this a few days ago and "revived" my old https://github.com/networkException/serenity/tree/honey-its-time-for-your-monthly-spec-rewrite branch, which implements large parts of the spec changes.

I have a working prototype, theres a bunch of cleanup and committing to do still though

@networkException networkException mentioned this issue Oct 30, 2023
Merged
@github-project-automation github-project-automation bot moved this from Todo to Done in Ladybird Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working fund This issue can be funded via polar.sh web compatibility
Projects
No open projects
Ladybird
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

LibJS+LibWeb: Bring script fetching closer to the spec networkException/serenity
4 participants
@awesomekling @TobyAsE @krkk @networkException