RESOLVED FIXED 83758
Web Inspector: Node Value should not get corrupt while jump over different matches in search.
https://bugs.webkit.org/show_bug.cgi?id=83758
Summary Web Inspector: Node Value should not get corrupt while jump over different ma...
sam
Reported 2012-04-12 01:20:04 PDT
Steps to reproduce : 1). Open html file, as an attachment. 2). Get to the inspect element view/ 3). Try for searching "we". 4). Jump over different search matches. 5). Observe the text value getting misplaced for anchor node. Expected Behavior: text value should not get misplacd
Attachments
sample.html for reproducing the bug. (119 bytes, text/html)
2012-04-12 01:36 PDT, sam
no flags
Patch (1.90 KB, patch)
2012-04-12 02:55 PDT, sam
no flags
Patch (1.95 KB, patch)
2012-04-12 03:45 PDT, sam
no flags
sam
Comment 1 2012-04-12 01:34:19 PDT
Constructing highlight node for a matched element requires 2 modes of operations. "added" -> for adding new span element for matched value with highlight class. "changed" -> for appending/manipulating prefix/suffix node to highlight node. There are cases were there will be multiple instances of matches in a node. This will follow the same mode of operations for other matches in a node, but now on manipulated nodes. While exec updateSearchHighlight, for hiding the highlights for cached results, the order in which updateEntryHide is called should then work in rev order to preserve the semantic of node. I am creating a patch for it. I will upload it soon!!
sam
Comment 2 2012-04-12 01:36:18 PDT
Created attachment 136851 [details] sample.html for reproducing the bug.
sam
Comment 3 2012-04-12 02:55:49 PDT
Alexander Pavlov (apavlov)
Comment 4 2012-04-12 03:11:02 PDT
Comment on attachment 136861 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136861&action=review > Source/WebCore/inspector/front-end/ElementsTreeOutline.js:640 > + if (show) { Since this now falls into two branches, it would be better to remove the |updater| var (which was introduced to avoid branching) altogether and call updateEntryShow and updateEntryHide directly in the branches instead.
sam
Comment 5 2012-04-12 03:45:39 PDT
sam
Comment 6 2012-04-12 03:47:35 PDT
(In reply to comment #4) > (From update of attachment 136861 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=136861&action=review > > > Source/WebCore/inspector/front-end/ElementsTreeOutline.js:640 > > + if (show) { > > Since this now falls into two branches, it would be better to remove the |updater| var (which was introduced to avoid branching) altogether and call updateEntryShow and updateEntryHide directly in the branches instead. Thank you Alexander for review! I have uploaded the patch.
Alexander Pavlov (apavlov)
Comment 7 2012-04-12 04:27:04 PDT
Comment on attachment 136872 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136872&action=review I believe it's ready for landing! > Source/WebCore/ChangeLog:7 > + highlightResults is changed in "last changed first corrected" order to preserve the semantic of node. optional: I'd replace the word "semantic" by "structure" for clarity. > Source/WebCore/inspector/front-end/ElementsTreeOutline.js:637 > + // Preserve the semantic of node by following the order of updates for hide and show. Ditto.
WebKit Review Bot
Comment 8 2012-04-12 06:40:11 PDT
Comment on attachment 136872 [details] Patch Clearing flags on attachment: 136872 Committed r113970: <http://trac.webkit.org/changeset/113970>
WebKit Review Bot
Comment 9 2012-04-12 06:40:17 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.