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.

Sign up for GitHub

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

Jump to bottom

fix: the first LF following pre and textarea tag should be ignored, fix #5992 #6022

Merged
merged 5 commits into from
Jul 10, 2017
Merged

fix: the first LF following pre and textarea tag should be ignored, fix #5992 #6022

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9950f3
fix: The first lf following the pre and textarea shuold be ignored.
defcc Jul 3, 2017
e7a5764
test: add proper test case
defcc Jul 3, 2017
992d902
refactor: remove useless toLowerCase check
defcc Jul 4, 2017
94b4eb7
Update html-parser.js
yyx990803 Jul 10, 2017
962b614
Update parser.spec.js
yyx990803 Jul 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions src/compiler/parser/html-parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ const decodingMap = {
const encodedAttr = /&(?:lt|gt|quot|amp);/g
const encodedAttrWithNewLines = /&(?:lt|gt|quot|amp|#10);/g

// #5992
const ignoreFirstLFTagList = makeMap('pre,textarea', true)
const isIgnoreFirstLf = (tag, html) => tag && ignoreFirstLFTagList(tag.toLowerCase()) && html[0] === '\n'
Copy link
Member

@javoski javoski Jul 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Not need to call toLowerCase() here, and it seems <textarea> will never reach to this function since isPlainTextElement('textarea') equals true.

Copy link
Member Author

@defcc defcc Jul 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

  1. html5 tag names are case-insensitive, so we use lower case to compare
  2. isIgnoreFirstLf will be invoked when in textarea parse https://github.com/vuejs/vue/pull/6022/files#diff-084898b8c9f6d4d2833b18312180ef8fR172

Copy link
Member

Choose a reason for hiding this comment

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

  1. It'll call toLowerCase() when passing true to makeMap as the second param.
  2. I didn't notice those changes starting from line 172, my fault😂.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Thanks, I missed it, I'll update.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


function decodeAttr (value, shouldDecodeNewlines) {
const re = shouldDecodeNewlines ? encodedAttrWithNewLines : encodedAttr
return value.replace(re, match => decodingMap[match])
Expand All @@ -75,6 +79,9 @@ export function parseHTML (html, options) {
last = html
// Make sure we're not in a plaintext content element like script/style
if (!lastTag || !isPlainTextElement(lastTag)) {
if (isIgnoreFirstLf(lastTag, html)) {
advance(1)
}
let textEnd = html.indexOf('<')
if (textEnd === 0) {
// Comment:
Expand Down Expand Up @@ -152,16 +159,19 @@ export function parseHTML (html, options) {
options.chars(text)
}
} else {
var stackedTag = lastTag.toLowerCase()
var reStackedTag = reCache[stackedTag] || (reCache[stackedTag] = new RegExp('([\\s\\S]*?)(</' + stackedTag + '[^>]*>)', 'i'))
var endTagLength = 0
var rest = html.replace(reStackedTag, function (all, text, endTag) {
let endTagLength = 0
const stackedTag = lastTag.toLowerCase()
const reStackedTag = reCache[stackedTag] || (reCache[stackedTag] = new RegExp('([\\s\\S]*?)(</' + stackedTag + '[^>]*>)', 'i'))
const rest = html.replace(reStackedTag, function (all, text, endTag) {
endTagLength = endTag.length
if (!isPlainTextElement(stackedTag) && stackedTag !== 'noscript') {
text = text
.replace(/<!--([\s\S]*?)-->/g, '$1')
.replace(/<!\[CDATA\[([\s\S]*?)]]>/g, '$1')
}
if (isIgnoreFirstLf(stackedTag, text)) {
text = text.substring(1)
}
if (options.chars) {
options.chars(text)
}
Expand Down
18 changes: 16 additions & 2 deletions test/unit/modules/compiler/parser.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,21 @@ describe('parser', () => {
expect(span.children[0].text).toBe(' ')
})

// #5992
it('ignore the first LF in <pre> tag', function () {
const options = extend({}, baseOptions)
const ast = parse('<div><pre>\nabc</pre>\ndef<pre>\n\nabc</pre></div>', options)
const pre = ast.children[0]
expect(pre.children[0].type).toBe(3)
expect(pre.children[0].text).toBe('abc')
const text = ast.children[1]
expect(text.type).toBe(3)
expect(text.text).toBe('\ndef')
const pre2 = ast.children[2]
expect(pre2.children[0].type).toBe(3)
expect(pre2.children[0].text).toBe('\nabc')
})

it('forgivingly handle < in plain text', () => {
const options = extend({}, baseOptions)
const ast = parse('<p>1 < 2 < 3</p>', options)
Expand Down Expand Up @@ -530,8 +545,7 @@ describe('parser', () => {
expect(whitespace.children.length).toBe(1)
expect(whitespace.children[0].type).toBe(3)
// textarea is whitespace sensitive
expect(whitespace.children[0].text).toBe(`
<p>Test 1</p>
expect(whitespace.children[0].text).toBe(` <p>Test 1</p>
test2
`)

Expand Down