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

T - Code gutter is not rendering correctly when multiple MDN pages are open #7322

Closed
markwillis opened this issue Jul 8, 2020 · 15 comments
Closed
Assignees
Labels
p2 Medium Priority: One of the next sprints

Comments

@markwillis
Copy link

markwillis commented Jul 8, 2020

Summary

Code panel gutter is not displaying correctly when there are two tabs open with a code panel

Steps to reproduce

  1. Open an MDN page e.g https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice
  2. Open a different page e.g https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/slice
  3. Refresh one of the pages a couple of times
  4. Note the code gutter disappears, but code line numbers remain.

Actual behaviour

https://share.getcloudapp.com/2Nu59d1W

You can see that the code gutters are not rendering as expected when the page is refreshed. Happens occasionally when only one page open, but happens consistently when there are 2+ tabs open in MDN.

Spec

Chrome: Version 83.0.4103.116 (Official Build) (64-bit)
macOS Catalina 10.15.5

Also, thank you all for the truly awesome docs 😃

@markwillis markwillis added the status: needs triage Status: Untriaged label Jul 8, 2020
@tobinmori tobinmori added p2 Medium Priority: One of the next sprints and removed status: needs triage Status: Untriaged labels Jul 13, 2020
@tobinmori
Copy link

tobinmori commented Jul 13, 2020

Next step: timeboxed investigation for possible regression. Code mirror upgrade?

@tobinmori tobinmori added this to the Lima (S1 Q3 2020) milestone Jul 13, 2020
@chinikes chinikes changed the title Code gutter is not rendering correctly when multiple MDN pages are open T - Code gutter is not rendering correctly when multiple MDN pages are open Jul 15, 2020
@schalkneethling
Copy link

Update on this issue:

I have contacted Marijn, the creator of CodeMirror to see if the problem is something he is aware of, or have encountered in the past. I am also upgrading to CodeMirror 5.56.0 here mdn/bob#496

I have looked over the recent changelog[https://github.com/codemirror/CodeMirror/blob/master/CHANGELOG.md], but nothing jumps out at me as related except perhaps https://github.com/codemirror/CodeMirror/blob/master/CHANGELOG.md#5530-2020-04-21

But we should be on 5.55.0 in production

@tobinmori
Copy link

thanks @schalkneethling

@schalkneethling
Copy link

I have noticed that the last release of Bob was on the 1st of April 👀 This is because semantic-release was not generating an NPM release based on the pull requests we were merging aka. mostly dependency updates.

I have done some cleanup and switched to using a plain and simple Github Action[https://github.com/mdn/bob/pull/503] so, we will have a release soon that will then cause an update on the side of the interactive examples.

Once that is available, I will ask some folks to kick the tires and if all looks good we can merge. This will, in turn, trigger a production release that will then take with it the latest version of CodeMirror i.e. 5.56.0

@marijnh
Copy link

marijnh commented Jul 30, 2020

Reproducing this doesn't require two separate tabs for me. In Chrome, just opening one of those pages and refreshing a few times shows the problem.

The issue doesn't ring any bells. Upgrading CodeMirror sounds like a good idea. Calling refresh on the editor instance seems to fix the gutter, so it's possible that there is some race condition that causes the editor to sometimes be initialized before it is visible or fully styled (preventing it from measuring its DOM dimensions properly on init).

@schalkneethling
Copy link

Reproducing this doesn't require two separate tabs for me. In Chrome, just opening one of those pages and refreshing a few times shows the problem.

The issue doesn't ring any bells. Upgrading CodeMirror sounds like a good idea. Calling refresh on the editor instance seems to fix the gutter, so it's possible that there is some race condition that causes the editor to sometimes be initialized before it is visible or fully styled (preventing it from measuring its DOM dimensions properly on init).

Thank you for the feedback @marijnh - makes me wonder whether we should consider adding a 500ms timeout on DOM loaded and call refresh from within that? This might be related to this living inside an iframe.

@schalkneethling
Copy link

Merged mdn/interactive-examples#1631 - This will bring us to the latest version of CodeMirror. Once this is live in production I will see if I can still reproduce the problem.

@schalkneethling
Copy link

schalkneethling commented Aug 10, 2020

I am pretty(95%) confident the latest CodeMirror is on production now and I can still reproduce this problem. I am thinking our only way out of this, for now, is to add a 500ms timeout to the JS examples and call refresh on the editor as @marijnh also suggested in https://app.zenhub.com/workspaces/mdn-dev-5d6ea9fb3fe11605a63cea86/issues/mdn/kuma/7322#issuecomment-666167891

Thoughts @atopal

@tobinmori
Copy link

Gave @schalkneethling greenlight to add the refresh hack for now.

@schalkneethling
Copy link

PR on bob: mdn/bob#511

@schalkneethling
Copy link

I have tested and for me, the 500ms delayed refresh call resolves the problem. One can see there for a split second the problem is there, but then the refresh call resolves it without need a page refresh:

A animated gif showing the gutter problem being resolved of a 500ms wait

It is a bit jarring, to be honest, so, perhaps we should shorten it to 300ms? @escattone Thoughts?

@escattone
Copy link
Contributor

@schalkneethling Thanks. I agree, it is a bit jarring. How low can we take the timeout and still correct the problem? I think it's worth trying a lower timeout.

@schalkneethling
Copy link

@schalkneethling Thanks. I agree, it is a bit jarring. How low can we take the timeout and still correct the problem? I think it's worth trying a lower timeout.

Thankfully I can test this using Chrome devtools local overrides. Phew. Let me take a look.

@schalkneethling
Copy link

Reducing it to 200ms here: mdn/bob#522

@schalkneethling
Copy link

Deployed to production. @markwillis if this still presents a problem for you, please let us know and we can reopen this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2 Medium Priority: One of the next sprints
Projects
None yet
Development

No branches or pull requests

6 participants