asciidoctor / asciidoctor-browser-extension

:white_circle: An extension for web browsers that converts AsciiDoc files to HTML using Asciidoctor.js.
https://chrome.google.com/webstore/detail/asciidoctorjs-live-previe/iaalpfgpbocpdfblpnhhgllgbdbchmia
MIT License
218 stars 50 forks source link

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

Closed mminot-yseop closed 1 year ago

mminot-yseop commented 1 year ago

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 commented 1 year ago

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:

https://github.com/asciidoctor/asciidoctor-browser-extension/blob/fc63d9b7efc0a6c7a74c58387643b1f8311cda53/app/js/loader.js#L72-L74

ggrossetie commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

SunSerega commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

ggrossetie commented 1 year ago

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 commented 1 year ago

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 https://github.com/asciidoctor/asciidoctor-browser-extension/issues/646#issuecomment-1574849770) 😞

mminot-yseop commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

By the way, Firefox approved the new version

SunSerega commented 1 year ago

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 commented 1 year ago

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 🤔