Bug 83758 - Web Inspector: Node Value should not get corrupt while jump over different matches in search.
Summary: Web Inspector: Node Value should not get corrupt while jump over different ma...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-12 01:20 PDT by sam
Modified: 2012-04-12 06:40 PDT (History)
11 users (show)

See Also:


Attachments
sample.html for reproducing the bug. (119 bytes, text/html)
2012-04-12 01:36 PDT, sam
no flags Details
Patch (1.90 KB, patch)
2012-04-12 02:55 PDT, sam
no flags Details | Formatted Diff | Diff
Patch (1.95 KB, patch)
2012-04-12 03:45 PDT, sam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sam 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
Comment 1 sam 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!!
Comment 2 sam 2012-04-12 01:36:18 PDT
Created attachment 136851 [details]
sample.html for reproducing the bug.
Comment 3 sam 2012-04-12 02:55:49 PDT
Created attachment 136861 [details]
Patch
Comment 4 Alexander Pavlov (apavlov) 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.
Comment 5 sam 2012-04-12 03:45:39 PDT
Created attachment 136872 [details]
Patch
Comment 6 sam 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.
Comment 7 Alexander Pavlov (apavlov) 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.
Comment 8 WebKit Review Bot 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>
Comment 9 WebKit Review Bot 2012-04-12 06:40:17 PDT
All reviewed patches have been landed.  Closing bug.