ContentSearchUtilities lineEndings only supports \n. That causes issues with non-unix line endings since the front-end supports \n, \r or \r\n <rdar://problem/15829887>
<rdar://problem/16190305>
<rdar://problem/16190323>
Created attachment 225424 [details] Patch
Comment on attachment 225424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=225424&action=review r=me > Source/JavaScriptCore/ChangeLog:3 > + Imporve how ContentSearchUtilities::lineEndings works by supporting the three common line endings. Typo: Imporve > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:91 > size_t lineEnd = endings->at(lineNumber); > String line = text.substring(start, lineEnd - start); These variable names are misleading. endings->at(lineNumber) is actually giving you the index of the first character on lineNumber+1. Maybe that the way I think about it. I would expect "lineEnd" to be a character on this line, but here it is not. > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:116 > { > auto result = std::make_unique<Vector<size_t>>(); > > - unsigned start = 0; > + size_t start = 0; > while (start < text.length()) { > - size_t lineEnd = text.find('\n', start); > - if (lineEnd == notFound) > + size_t nextStart = text.findNextLineStart(start); > + if (nextStart == notFound) { > + result->append(text.length()); > break; > + } > > - result->append(lineEnd); > - start = lineEnd + 1; > + result->append(nextStart); > + start = nextStart; Alongside the earlier comment, the function is named "lineEndings" but it returns the indexes that start the next line / EOF. I'm fine with the patch as is, but it might be clearer with better names. > LayoutTests/inspector-protocol/debugger/searchInContent-linebreaks.html:65 > +Verify that Debugger.searchInContent works with scripts that have varying line endings. Nit: Double space "varying line endings"
Created attachment 225426 [details] Patch
Comment on attachment 225426 [details] Patch Clearing flags on attachment: 225426 Committed r164845: <http://trac.webkit.org/changeset/164845>
All reviewed patches have been landed. Closing bug.