Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Html syntax highlight bug#3271

Merged
jbhoosreddy merged 4 commits into
firefox-devtools:masterfrom
jbhoosreddy:html-syntax-highlight-bug
Jul 6, 2017
Merged

Html syntax highlight bug#3271
jbhoosreddy merged 4 commits into
firefox-devtools:masterfrom
jbhoosreddy:html-syntax-highlight-bug

Conversation

@jbhoosreddy
Copy link
Copy Markdown
Contributor

Associated Issue: #3222

Summary of Changes

  • fixes html syntax highlighting

Test Plan

Example test plan:

  • Test passing
  • Visual Inspection

Screenshots/Videos (OPTIONAL)

screen shot 2017-07-05 at 10 19 14 pm

@jbhoosreddy jbhoosreddy requested a review from jasonLaster July 6, 2017 02:21
Comment thread src/utils/source.js
return contentTypeModeMap["text/javascript"];
}

if (isHTMLLike) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a second check here because it can happen that we do have a contentType = "text/html" or it is equal to something else. In that case, we would still like to render it as htmlmixed.

I believe that this logic might be simplified if in devtools-core/devtools-source-maps we set html contentType to text/html for file extensions .html.

In fact, I'm not sure if there's any case for (!contentType && isHTMLLike) as far as I could tell in any codepath.

Copy link
Copy Markdown
Contributor

@codehag codehag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Comment thread src/utils/source.js

function getMode(source: Source) {
if (!source.text) {
const { contentType, text } = source;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jbhoosreddy
Copy link
Copy Markdown
Contributor Author

Since it's approved I'll go ahead and merge it. The failing CI test is only because of Percy. Rest of the tests pass and I double checked locally.

@jbhoosreddy jbhoosreddy merged commit 2e264c9 into firefox-devtools:master Jul 6, 2017
@jbhoosreddy jbhoosreddy deleted the html-syntax-highlight-bug branch July 6, 2017 15:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants