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

Web page code examples from prism are broken on last production build #6310

Closed
dgesteves opened this issue Jan 2, 2020 · 50 comments
Closed
Assignees
Labels
comp: frontend Component: Frontend p1 High Priority: Current sprint

Comments

@dgesteves
Copy link

Screenshot 2020-01-02 at 13 40 44

Screenshot 2020-01-02 at 13 40 51

Screenshot 2020-01-02 at 13 41 01

@peterbe
Copy link
Contributor

peterbe commented Jan 2, 2020

@schalkneethling I've seen that too. Do you know what it could be?

By the way:
Firefox Nightly
Screen Shot 2020-01-02 at 9 30 57 AM

Chrome 79
Screen Shot 2020-01-02 at 9 31 38 AM

Safari
Screen Shot 2020-01-02 at 9 32 45 AM

@tobinmori tobinmori transferred this issue from mdn/kuma Jan 6, 2020
@chrisdavidmills
Copy link
Contributor

@tobinmori this looks like a platform issue rather than a docs issue. See also #6307.

@tobinmori
Copy link

thanks @chrisdavidmills, sorry for the mis-assignment!

@tobinmori tobinmori transferred this issue from mdn/sprints Jan 7, 2020
@tobinmori tobinmori added comp: frontend Component: Frontend status: needs answer: product Status: Awaiting decision from product labels Jan 7, 2020
@tobinmori
Copy link

@atopal can you let me know the priority of this one?

@peterbe
Copy link
Contributor

peterbe commented Jan 7, 2020

What the heck. Now it works! 🙃
Screen Shot 2020-01-07 at 11 42 16 AM

I'm confident that there hasn't been a Kuma release since. And my Chrome is still version 79.

@tobinmori
Copy link

@EstevesDiogo - is this working for you?

Thanks!

@schalkneethling
Copy link

I can also confirm that this works for me in Fx-Dev, Fx, Chrome and Safari

@schalkneethling schalkneethling removed the status: needs answer: product Status: Awaiting decision from product label Jan 8, 2020
@atopal
Copy link
Contributor

atopal commented Jan 8, 2020

I've seen that before when we were experimenting with inline CSS. It seems like some CSS is missing. @tobinmori this is the top priority, if anyone can reproduce. It's the most visible feature on MDN.

@atopal
Copy link
Contributor

atopal commented Jan 8, 2020

I wanted to check when this broke and the page load waterfall on Speedcurve, but it seems like the last recorded deployment was on December 19. Is that correct?

@atopal
Copy link
Contributor

atopal commented Jan 8, 2020

Weird thing is that this doesn't show up in Speedcurve tests over the last few days (only testing in Chrome though). It seems to be an intermittent issue, so it could still be happening. Do we have any error logging that would help to see if anything has spiked?

@tobinmori tobinmori added the p1 High Priority: Current sprint label Jan 8, 2020
@schalkneethling
Copy link

Weird thing is that this doesn't show up in Speedcurve tests over the last few days (only testing in Chrome though). It seems to be an intermittent issue, so it could still be happening. Do we have any error logging that would help to see if anything has spiked?

If it is some CSS not loading, which is surely what it looks like, then it is happening on the sub-domain(interactive-examples.mdn.mozilla.net) and not the main d.m.o domain so, not sure what logging we have there.

@peterbe
Copy link
Contributor

peterbe commented Jan 8, 2020

The only CSS that matters is editor-js.css and codemirror.css. If you block those URLs in the dev tools, it doesn't reproduce. It just goes horribly wrong. So wrong that it's not what happened.

What's so baffling is that the last release was: fb2d23f which was 3 weeks ago. What it introduced has virtually nothing to do with CSS or .css files or anything like that.

Even after months on the team I still don't really understand how interactive examples really works but someone more familiar with how it's released and deployed and hosted. For one thing, the static .css files aren't aggressive cached so what might happen if one changes in the CDN and thee other doesn't or something like that?

@dgesteves
Copy link
Author

@tobinmori It works and then it does not, it is
ScreenRecording2020-01-09at19154

@peterbe
Copy link
Contributor

peterbe commented Jan 9, 2020

Thank you!! Do you see anything interesting in the Web Console when that happens?
And does it go away when you refresh the page?

@schalkneethling schalkneethling added this to To do in Developer Insights via automation Jan 10, 2020
@dgesteves
Copy link
Author

dgesteves commented Jan 10, 2020

Hey, @peterbe yes some times when you refresh it corrects and some times when you refresh it breaks again as you can see on this gif ---->
ScreenRecording2020-01-10at16261
Screenshot 2020-01-10 at 17 00 15

@peterbe
Copy link
Contributor

peterbe commented Jan 10, 2020

It's extremely hard to see but all that yellow text appears to be the same block of yellow text on the renders when it actually does work.

@dgesteves
Copy link
Author

dgesteves commented Jan 10, 2020

Yes, they are the same warnings.
Just updated the prev. comment with the console output.

@schalkneethling
Copy link

Yes, they are the same warnings.
Just updated the prev. comment with the console output.

Interestingly I see all of those, except for the first, mentioned Codota.com. Is there perhaps a browser extension from this website/service that you have installed?

If so, could you try disabling it, or accessing the site in a private window and see whether the problem goes away? Thanks so much for your help in debugging this.

@tobinmori
Copy link

tobinmori commented Jan 13, 2020

Btw, @schalkneethling - @atopal and I could reliably reproduce this upon opening a new tab in Chrome and loading the page from scratch. (if you reload an existing instance of the page in a tab, the err goes away).

A hypothesis - some sort of Chrome-specific race condition regarding CSS loading/rendering related to caching?

I could not reproduce this in FF or Safari.

@schalkneethling
Copy link

We have updated CodeMirror and there is a new release of Bob:
mdn/interactive-examples#1525

I will do some testing locally to ensure nothing broke but I have also asked @wbamberg to kick the tyres.

@schalkneethling
Copy link

Ok, mdn/interactive-examples#1525 has been merged. In ~30min the new builds of interactive examples will be on production. 🤞

@wbamberg
Copy link

The new version of CodeMirror is live, and seems to work fine, but I still see the overlapping-line-numbers problem in Chrome :(.

@peterbe
Copy link
Contributor

peterbe commented Jan 14, 2020

The new version of CodeMirror is live, and seems to work fine, but I still see the overlapping-line-numbers problem in Chrome :(.

Can you please verify the network (JS files) used inside the iframe?

As of 1min ago I see /js/codemirror-5-50-2.js which is the new CodeMirror build.

@peterbe
Copy link
Contributor

peterbe commented Jan 14, 2020

Screen Shot 2020-01-14 at 1 42 08 PM

:(

I'm not 100% sure if I'm using Chrome right.
I was definitely able to reproduce it, as you can see, but when you "Open in a new Tab" it won't start with the Web Console open. And refreshing would solve it which makes it impossible to know if it solved or not solved it.

@wbamberg
Copy link

The new version of CodeMirror is live, and seems to work fine, but I still see the overlapping-line-numbers problem in Chrome :(.

Can you please verify the network (JS files) used inside the iframe?

Screen Shot 2020-01-14 at 10 55 07 AM

when you "Open in a new Tab" it won't start with the Web Console open. And refreshing would solve it which makes it impossible to know if it solved or not solved it.

https://stackoverflow.com/questions/12212504/automatically-open-chrome-developer-tools-when-new-tab-new-window-is-opened/36957422#36957422

@tobinmori
Copy link

tobinmori commented Jan 14, 2020

+1 Confirming that the err still occurs, unfortunately; I'm using codemirror build 5-50-2. (Chrome)

@peterbe
Copy link
Contributor

peterbe commented Jan 14, 2020

SOME PROGRESS! Hang on this is messy.

So, I manually edited the compiled browser JS (used prettier to make it slightly easier) so that when the e = CodeMirror(document.querySelector('#editor'), ...) gets created I injected something like this: e = window.CODEMIRROR = CodeMirror(document.querySelector('#editor'), ...). Now, after some crazy cache invalidation, and eventual reproduce in Chrome, I can change the context (in Chrome's dev tools) to the context of the iframe and have access to window.CODEMIRROR. Calling window.CODEMIRROR.refresh() fixes the gutter!!

Repro
Screen Shot 2020-01-14 at 3 01 51 PM

Fix
Screen Shot 2020-01-14 at 3 02 14 PM

That fixes it!
See codemirror/codemirror5#4412 (comment) for the root of the clue.

According to https://codemirror.net/doc/manual.html

cm.refresh()
If your code does something to change the size of the editor element (window resizes are already listened for), or unhides it, you should probably follow up by calling this method to ensure CodeMirror is still looking as intended. See also the autorefresh addon.

I have a feeling that https://codemirror.net/doc/manual.html#addon_autorefresh might be the ticket. It's kinda hard to know exactly how our iframe works but clearly there's something weird going on exclusively in some browsers (looking at you Chrome).

@schalkneethling
Copy link

Turns out we are already doing that for the tabbed interface:
https://github.com/mdn/bob/blob/master/editor/js/editor-libs/tabby.js#L167

Definitely looks like autorefresh will make this easier by taking care of that automatically. Thanks, @peterbe

@atopal
Copy link
Contributor

atopal commented Jan 15, 2020

Great find Peter! \o/

@wbamberg
Copy link

Turns out we are already doing that for the tabbed interface:
https://github.com/mdn/bob/blob/master/editor/js/editor-libs/tabby.js#L167

That's a bit worrying, because I see this problem with the HTML examples, too:

Screen Shot 2020-01-15 at 7 29 52 AM

@peterbe
Copy link
Contributor

peterbe commented Jan 15, 2020

That means that how we're doing .refresh() in tabby.js isn't working. Anyway. The change I introduced only fixed the JS editor. I guess we need to re-evaluate and try to add that addon to all editors.

@schalkneethling
Copy link

Turns out we are already doing that for the tabbed interface:
https://github.com/mdn/bob/blob/master/editor/js/editor-libs/tabby.js#L167

That's a bit worrying, because I see this problem with the HTML examples, too:

So, we are not doing that for the initial load. If you switch between the tabs does it fix itself?

@schalkneethling
Copy link

I can unfortunately still reproduce this on production in Chrome. While it does not happen on the first load, refreshing the page in the new tab a couple of times(4-5) reproduces the issues. 😿

https://www.loom.com/share/b881f444e82148f5a69f95ce51c26cb6

@atopal
Copy link
Contributor

atopal commented Jan 20, 2020

@schalkneethling on which page are you seeing that behavior? I was able to reproduce it reliably, but can't reproduce it at all anymore in the latest release version of Chrome (79)

@schalkneethling
Copy link

@schalkneethling on which page are you seeing that behavior? I was able to reproduce it reliably, but can't reproduce it at all anymore in the latest release version of Chrome (79)

I followed the steps outlined by Will on the related issue:

@atopal
Copy link
Contributor

atopal commented Jan 20, 2020

Ah, alright. I thought this was the "open new tab" > "open MDN page" steps. Which wasn't reproducing for me anymore.

That said, even the steps you just outlined, I can't reproduce it.

@tobinmori
Copy link

Unfortunately, I can reproduce this; initial load is fine, but the gutter bug appears after I hit reload on the right-clicked new tab. Chrome 79.

@atopal
Copy link
Contributor

atopal commented Jan 21, 2020

Almost definitely a race condition. I can now reproduce again in about 20% of the reloads. I think this is fine for now, but I'd like to know if there is any way we can instrument this. I'd really like to know how often this is happening. Any ideas?

@schalkneethling
Copy link

I and @peterbe chatted about this briefly. The other option is to not use the plugin, and instead do something like:

onload = function() {
  setTimeout(() => {
      editor.refresh();
  }, 60);
}

The problem I see with instrumenting this is to know what to use as an event to send a GA event. There is no error that correlates with this happening so, it is hard to know when exactly it happened.

Perhaps there is a class or DOM element that is not injected and one can check for the presence/absence of it. At the same time, perhaps it is overkill and the times this does happen would be very rare now?

@atopal
Copy link
Contributor

atopal commented Jan 26, 2020

Well, the whole point is that we don't know whether it's rare or not, because we have no idea what triggers it. Maybe we should add a small feedback option, and ask users to report broken examples. We don't have a baseline to measure it against, but at least we could get a hint for the magnitude.

@peterbe
Copy link
Contributor

peterbe commented Jan 26, 2020 via email

@tobinmori
Copy link

@atopal can we close or table this for now?

@atopal
Copy link
Contributor

atopal commented Feb 3, 2020

yeah, let's close this :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
comp: frontend Component: Frontend p1 High Priority: Current sprint
Projects
No open projects
Development

No branches or pull requests

7 participants