Bug 159110

Summary: REGRESSION: Web Inspector: Text search broken in resources with <CR>
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, saam, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix bburg: review+

Joseph Pecoraro
Reported 2016-06-24 18:00:00 PDT
Summary: Text search broken in resources with <CR>. Steps To Reproduce: 1. Inspect <http://daringfireball.net> 2. Open main resource (daringfireball.net) in Resources tab 3. Click in the editor 4. ⌘F to show find banner 5. Search for "href" or "title=" => few results, all broken after the first <CR> Notes: - We changed CodeMirror to only \n as lineEndings earlier. The backend still treats \r as a line ending, which is causing slow drifting in line number differences between the frontend and backend. 
Attachments
[PATCH] Proposed Fix (5.14 KB, patch)
2016-06-24 18:19 PDT, Joseph Pecoraro
bburg: review+
Joseph Pecoraro
Comment 1 2016-06-24 18:00:14 PDT
Joseph Pecoraro
Comment 2 2016-06-24 18:19:46 PDT
Created attachment 282040 [details] [PATCH] Proposed Fix
Blaze Burg
Comment 3 2016-06-25 09:49:46 PDT
Comment on attachment 282040 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=282040&action=review r=me with comments > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:-107 > - size_t nextStart = text.findNextLineStart(start); Does this still have other callers? > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:108 > + if (nextStart == notFound || nextStart == text.length()) { Good catch, although wouldn't the last character be text.length() - 1?
Joseph Pecoraro
Comment 4 2016-06-27 11:12:27 PDT
(In reply to comment #3) > Comment on attachment 282040 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=282040&action=review > > r=me with comments > > > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:-107 > > - size_t nextStart = text.findNextLineStart(start); > > Does this still have other callers? Hmm, no it doesn't. > > Source/JavaScriptCore/inspector/ContentSearchUtilities.cpp:108 > > + if (nextStart == notFound || nextStart == text.length()) { > > Good catch, although wouldn't the last character be text.length() - 1? Yes! Good catch.
Joseph Pecoraro
Comment 5 2016-06-27 11:19:32 PDT
Note You need to log in before you can comment on or make changes to this bug.