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.

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

fix(split links): run in iframes in Chrome and Violentmonkey Beta #504

Merged
merged 1 commit into from Jun 27, 2022

Conversation

ROpdebee
Copy link
Owner

@ROpdebee ROpdebee commented Jun 21, 2022

Fixes issue reported by chaban in the forums.

We already knew that Firefox doesn't fire the DOMContentLoaded event, which is why we were listening to the iframe window's load event. However, it turns out that Chrome doesn't fire that event. I also tried mutation observers, they don't work either. I'm not sure whether Chrome fires the DOMContentLoaded event, but considering that the iframe's document readyState attribute is set to 'complete' immediately makes me doubt that. So we'll resort to the simplest approach: Continuous polling (eww!). Turns out the simple solution is just to listen for the load event on the iframe itself.

I'm not 100% certain the issue chaban is seeing, but I couldn't reproduce it on Firefox, and it was broken in Chrome either way, so it needed fixing. I'll ask chaban to test this as well to verify that the problem is solved.

We'll also need to apply this to Work code toolbox in #499.

@ROpdebee ROpdebee added bug Something isn't working mb_multi_external_links labels Jun 21, 2022
@ROpdebee ROpdebee requested a review from kellnerd June 21, 2022 19:52
@ROpdebee
Copy link
Owner Author

/deploy-preview for testing.

github-actions bot added a commit that referenced this pull request Jun 21, 2022
fix(split links): run in iframes in Chrome (#504)
@github-actions
Copy link

github-actions bot commented Jun 21, 2022

This PR changes 1 built userscript(s):

See all changes

@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #504 (02b5973) into main (73ddc60) will decrease coverage by 0.24%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##             main     #504      +/-   ##
==========================================
- Coverage   99.91%   99.67%   -0.25%     
==========================================
  Files          53       53              
  Lines        1228     1230       +2     
  Branches      195      196       +1     
==========================================
- Hits         1227     1226       -1     
- Misses          0        3       +3     
  Partials        1        1              
Impacted Files Coverage Δ
src/lib/util/dom.ts 88.88% <40.00%> (-11.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 73ddc60...02b5973. Read the comment docs.

@ROpdebee
Copy link
Owner Author

/deploy-preview new fix

github-actions bot added a commit that referenced this pull request Jun 22, 2022
fix(split links): run in iframes in Chrome (#504)
Chrome does not fire the window load event on iframe windows, Firefox
doesn't fire the DOMContentLoaded event. Instead, we should just listen
for the iframe load event itself, rather than load events on the contained
window or document.
@ROpdebee
Copy link
Owner Author

/deploy-preview again, better approach. Not sure where my head is at these days.

github-actions bot added a commit that referenced this pull request Jun 22, 2022
fix(split links): run in iframes in Chrome (#504)
@ROpdebee ROpdebee changed the title fix(split links): run in iframes in Chrome fix(split links): run in iframes in Chrome and Violentmonkey Beta Jun 22, 2022
@ROpdebee
Copy link
Owner Author

I think chaban confirmed that the last attempt did fix the problem. This fix should go into the work code toolbox in #499 too.

@ROpdebee ROpdebee merged commit 42e02e0 into main Jun 27, 2022
@ROpdebee ROpdebee deleted the split-links-iframes-chrome branch June 27, 2022 10:18
github-actions bot added a commit that referenced this pull request Jun 27, 2022
fix(split links): run in iframes in Chrome and Violentmonkey Beta (#504)
@github-actions
Copy link

🚀 Released 1 new userscript version(s):

  • mb_multi_external_links 2022.6.27 in dd420dd

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

Successfully merging this pull request may close these issues.

None yet

1 participant