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

injected script breaks browser default XML presentation #1136

Closed
ghost opened this issue Jan 20, 2020 · 6 comments · Fixed by #1137
Closed

injected script breaks browser default XML presentation #1136

ghost opened this issue Jan 20, 2020 · 6 comments · Fixed by #1137

Comments

@ghost
Copy link

ghost commented Jan 20, 2020

As a developer, I use web-browsers to visually inspect XML. Ember inspector injects a script tag that breaks the default browser presentation of the XML.

Issue and Steps to Reproduce

I believe this occurs because we indiscriminately append the boot <script>:

// TODO: make this conditional, only do it when requested.

Screenshots

unexpected

Screen Shot 2020-01-20 at 16 22 06

expected

Screen Shot 2020-01-20 at 16 21 18

@chancancode
Copy link
Member

Hm we should definitely check the document type and inject it conditionally!

@RobbieTheWagner
Copy link
Member

@chancancode better yet, we should only run inspector when you click the inspector tab.

@chancancode
Copy link
Member

As things are designed today, this particular script tag has to be injected eagerly to allow the inspector to possibly run later. We can add UI to change which sites to run on, but unless we require a refresh to activate the inspector this isn’t really avoidable at the moment.

@RobbieTheWagner
Copy link
Member

@chancancode yeah not sure if there is a good solution, but it would be ideal. There have been several reports of this breaking non-Ember sites in some way.

@ghost
Copy link
Author

ghost commented Jan 21, 2020

For this case, I'm entirely happy with inferring the mime type of the page.

@chancancode
Copy link
Member

@efx that’s probably the fix we can do in the medium term. I don’t know if chrome/firefox let you request that a content script to be injected only on html pages in the manifest. If they do, that’s great. If not, it’s not a big deal, we would just have to detect the document type in the content script and guard the script tag injection accordingly. If you or someone else is able to send a PR, it should be a fairly straightforward fix. Otherwise I’ll do it when I get back from vacation next week.

chancancode pushed a commit that referenced this issue Jan 28, 2020
Fix #1136.
I chose to whitelist the [HTML mime
type](https://www.iana.org/assignments/media-types/media-types.xhtml#text) only since it is simpler.
Per Godfrey's comment, we could declare [matching in the manifest](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) but it does so by URL or protocol. Because of that I think checking the mime type in the document is simpler and more readable.
wycats pushed a commit that referenced this issue Feb 20, 2020
Fix #1136.
I chose to whitelist the [HTML mime
type](https://www.iana.org/assignments/media-types/media-types.xhtml#text) only since it is simpler.
Per Godfrey's comment, we could declare [matching in the manifest](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) but it does so by URL or protocol. Because of that I think checking the mime type in the document is simpler and more readable.

(cherry picked from commit 87f8059)
nummi pushed a commit to nummi/ember-inspector that referenced this issue Apr 1, 2020
Fix emberjs#1136.
I chose to whitelist the [HTML mime
type](https://www.iana.org/assignments/media-types/media-types.xhtml#text) only since it is simpler.
Per Godfrey's comment, we could declare [matching in the manifest](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) but it does so by URL or protocol. Because of that I think checking the mime type in the document is simpler and more readable.
nummi pushed a commit to nummi/ember-inspector that referenced this issue Apr 5, 2020
Fix emberjs#1136.
I chose to whitelist the [HTML mime
type](https://www.iana.org/assignments/media-types/media-types.xhtml#text) only since it is simpler.
Per Godfrey's comment, we could declare [matching in the manifest](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) but it does so by URL or protocol. Because of that I think checking the mime type in the document is simpler and more readable.
patricklx pushed a commit to patricklx/ember-inspector that referenced this issue Sep 19, 2022
Fix emberjs#1136.
I chose to whitelist the [HTML mime
type](https://www.iana.org/assignments/media-types/media-types.xhtml#text) only since it is simpler.
Per Godfrey's comment, we could declare [matching in the manifest](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Match_patterns) but it does so by URL or protocol. Because of that I think checking the mime type in the document is simpler and more readable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants