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

Conflict with GitHub’s “New Code View” feature (initially beta, now prod-ish) #643

Closed
mminot-yseop opened this issue May 5, 2023 · 22 comments · Fixed by #645
Closed
Labels

Comments

@mminot-yseop
Copy link

mminot-yseop commented May 5, 2023

Hi.

When the “New Code Search and Code View” feature preview is enabled in GitHub, trying to open an AsciiDoc file by directly opening a URL (for example by picking one in my web browser’s history in a brand new browser tab) rather by than browsing a tree and clicking a GitHub link leads me to a raw JSON payload. The page displays correctly very briefly then shifts to the JSON.

It seems that this is actually caused by the AsciiDoctor extension trying to render something as if it were a piece of AsciiDoc source. It is even updated periodically:

Example:
loading

The issue suddenly popping up when the “Asciidoctor.js Live Preview” extension is enabled:
extension

The displayed JSON being periodically refreshed:
json-changing

As far as I can tell, this happens on every AsciiDoc file on GitHub. But again, you have to directly open the URL, not reach the file through a GitHub-internal link.

I contacted GitHub’s support about this and they asked me to make a report about this issue here.

Firefox 112.0.2
Xubuntu 20.04
Asciidoctor.js Live Preview 2.7.0
@ggrossetie
Copy link
Member

Indeed, I can reproduce this conflict.
I think the problem is that the response is application/json so we should also not enable the extension in this case:

if (Converter.isHtmlContentType(request)) {
return
}

@ggrossetie
Copy link
Member

ggrossetie commented May 6, 2023

This is worst than that, the content-type is text/plain:

image

but the content is JSON:

image

Since the URL ends with .adoc and the content type is text/plain, the extension assumes that this page serves a raw AsciiDoc document.

The only workaround I can think of is to disable the extension on the github.com domain 😞

@mminot-yseop
Copy link
Author

Thanks for the quick reply. I think they had similar issues in the past due to wrong Content Type headers; perhaps they’ll be able to fix their new code view. It’s still in beta for a reason, I guess. I’ll tell them about your answer.

The only workaround I can think of is to disable the extension on the github.com domain

Is that even possible? I don’t see anything like a blacklist in the Preferences tab of the Firefox extension.
For now, I’ll keep the GitHub feature preview disabled.

@ggrossetie
Copy link
Member

Is that even possible? I don’t see anything like a blacklist in the Preferences tab of the Firefox extension.
For now, I’ll keep the GitHub feature preview disabled.

I can declare an exception in the manifest but it would be better if GitHub could fix the content-type.

@mminot-yseop
Copy link
Author

I can declare an exception in the manifest but it would be better if GitHub could fix the content-type.

I think they will, as long as it’s clear that the current behavior of their code view in that regard is not “normal”. Releasing a version of the extension just for that may do more harm than good.

@jcansdale
Copy link

@ggrossetie,

This is worst than that, the content-type is text/plain

I tried to reproduce this using Chrome's inspect functionality, but the .adoc page appears to show text/html rather than text/plain.

image

I wonder why we're seeing different things?

@mminot-yseop
Copy link
Author

I see text/html as well. Strange.
The time, long ago, with the old code view, when something similar happened, I could clearly see that the Content Type was different according to whether the file was opened via a GitHub-internal link (OK) or directly in a brand new tab (not OK).

Sidenote: To my surprise, it seems that despite the overwhelming amount of not-so-good feedback, the new code view started becoming the default a few hours ago, with no visible way to disable it (but I may be overlooking something). So for now I’ll disable the extension and have to rely on IDEs to preview AsciiDoc files. 😢
(That’s strange: If I go look at public files in a private browsing session, I get the old code view, but otherwise I get the new one. 🤔)

@ggrossetie
Copy link
Member

I don't know how GitHub does this but they return a different response when the query is sent using XMLHttpRequest or fetch.

In the "Developer Tools":

> await fetch('https://github.com/yuzutech/kroki/blob/main/README.adoc').then(async (r) => r.headers.get('Content-Type') + '\n' + await r.text())
'text/plain; charset=utf-8\n{"payload":{"allShortcutsEnabled":true,"fileTree":{"":{"items":[{"name":".github","path":".github","contentType":"directory"},{"name":".mvn","path":".mvn","contentType":"directory"},{"name":"blockdiag","path":"blockdiag","contentType":"directory"},{"name":"bpmn","path":"bpmn","contentType":"directory"},{"name":"bytefield","path":"bytefield","contentType":"directory"}

If I add an Accept: text/plain (instead of the default value */*) then I get:

'text/html; charset=utf-8\n\n\n\n\n\n\n<!DOCTYPE html>\n<html lang="en" data-color-mode="auto" data-light-theme="light" data-dark-theme="dark" data-a11y-animated-images="system">\n  <head>\n    <meta charset="utf-8">\n  <link rel="dns-prefetch" href="https://github.githubassets.com">\n  <link rel="dns-prefetch" href="https://avatars.githubusercontent.com">\n  <link rel="dns-prefetch" href="https://github-cloud.s3.amazonaws.com">\n  <link rel="dns-prefetch" href="https://user-images.githubusercontent.com/">\n  <link rel="preconnect" href="https://github.githubassets.com" crossorigin>\n  <link rel="preconnect" href="https://avatars.githubusercontent.com">\n\n  \n\n\n  <link crossorigin="anonymous" media="all" rel="styl....

Which is funny because now I get HTML 🤷🏻

@mminot-yseop
Copy link
Author

Which is funny because now I get HTML 🤷🏻

The first time I tried to display https://github.com/yuzutech/kroki/blob/main/README.adoc, I got a “unicorn” error page from GitHub; maybe the HTML was this. 😆

@jcansdale
Copy link

I'm a little out of my depth with how this all works. Is there any specific feedback you think might be useful to the GitHub team responsible for this. Do you think there's a bug with how the content type is being reported?

@ggrossetie
Copy link
Member

Do you think there's a bug with how the content type is being reported?

Yes.
Technically, using text/plain and sending JSON is not wrong but misleading/inaccurate since application/json exists.

When the request header specifically ask text/plain it should either return a 406 (https://developer.mozilla.org/en/docs/Web/HTTP/Status/406) or a text/plain response. Here the response is text/html whereas the client specifically told the server that it can only process/understand text/plain.

In my opinion, the correct behavior should be:

  • Accept: */*: default content (HTML ?)
  • Accept: text/plain -> 406 Not Acceptable or default content (HTML ?)
  • Accept: text/html -> 'text/html; charset=utf-8 <!DOCTYPE ...
  • Accept: application/json -> 'application/json; charset=utf-8 {"payload":...

ggrossetie added a commit to ggrossetie/asciidoctor-chrome-extension that referenced this issue May 12, 2023
ggrossetie added a commit that referenced this issue May 13, 2023
@SunSerega
Copy link

I'm probably missing something obvious but... How do I get this fix applied? The latest release is more than 2 years old. So I guess there is some other place to get a version of this extension with the latest changes?

@mminot-yseop
Copy link
Author

I'm probably missing something obvious but... How do I get this fix applied? The latest release is more than 2 years old. So I guess there is some other place to get a version of this extension with the latest changes?

I got it to work via https://github.com/asciidoctor/asciidoctor-browser-extension/#firefox-1 but it has to be done again every time you close and reopen the browser. I hope there’ll be a release someday. Not sure GitHub will do anything on their side, with the huge number of issues pertaining to that new code view and the fact that they saw that this extension-side fix popped up.

@SunSerega
Copy link

I see, so only manually for now... Well, thanks for the quick reply anyway.

@mminot-yseop mminot-yseop changed the title Conflict with GitHub’s “New Code View” feature (available as beta) Conflict with GitHub’s “New Code View” feature (initially beta, now prod-ish) May 24, 2023
@ggrossetie
Copy link
Member

Sorry about the delay!
Chrome is pushing on manifest v3 but Firefox is not (completely) compatible... 😞

I will try to publish a hot-fix version using manifest v2 because upgrading to manifest v3 means that I need to maintain two versions of the codebase to support both Firefox and Chrome-based browsers (Chrome, Edge...)

@ggrossetie
Copy link
Member

Version 2.7.1 was published on Opera, Chrome and Firefox (and will hopefully will available soon).
You can also install it from the release page: https://github.com/asciidoctor/asciidoctor-browser-extension/releases/tag/v2.7.1

Unfortunately, Edge requires to migrate to manifest v3 (see #646 (comment)) 😞

@mminot-yseop
Copy link
Author

will hopefully will available soon

Naive question: How long does it usually take? Firefox still seems oblivious to the existence of the new version: https://addons.mozilla.org/en-US/firefox/addon/asciidoctorjs-live-preview/

@ggrossetie
Copy link
Member

Firefox is doing a manual review even for extensions that have high user ratings and/or when the changes are minimal...
In the past, it could take weeks or even months!

I would recommend installing the extension from the xpi file (attached in the release page).

@mminot-yseop
Copy link
Author

OK 😆 Done, thanks. First time installing an extension this way.
I can confirm the fix works. Glad to be able to enable the extension again.

@ggrossetie
Copy link
Member

By the way, Firefox approved the new version

@SunSerega
Copy link

SunSerega commented Jun 18, 2023

Btw, Opera extension page says the extension is published there (copied from Chrome extensions I guess?) without review. So it's kind of weird that it's the last to update...
Maybe something is needed for it to realize it needs to update?

@ggrossetie
Copy link
Member

Your extension needs to be manually reviewed and moderated before it can be added to Opera Stable. It is currently only viewable and available for download in Opera Developer and Opera Beta.

Maybe it's my first submission since they enabled auto-publishing 🤔

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

Successfully merging a pull request may close this issue.

4 participants