Keeps the original text node when search highlighting happens so it can later be restored.

Keeps the original text node when search highlighting happens so it can later be restored.

Previously, the search highlighting logic got rid of the original text node. That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/ea2290c7cb208a3575d1e2589da2fda9450491ab
Cr-Commit-Position: refs/heads/master@{#420906}

[Moe, 2016-09-23 15:08:56]

Description was changed from

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

BUG=639259
==========

to

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Moe, 2016-09-23 15:09:02]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 15:09:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Moe, 2016-09-23 15:10:38]

Description was changed from

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

Previously, the search highlighting logic got rid of the original text node.
That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-23 15:22:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-23 15:22:50]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Moe, 2016-09-23 17:18:04]

mahmadi@chromium.org changed reviewers:
+ dpapad@chromium.org

[Moe, 2016-09-23 17:18:05]

Hi Demetrios, Please take a look at this CL.

[dpapad, 2016-09-23 18:29:14]

On 2016/09/23 at 17:18:05, mahmadi wrote:
> Hi Demetrios, Please take a look at this CL.

I don't think the reported issue has anything to do with the searching
algorithm. When you drag and drop text from one input to another, the original
input becomes empty (happens both for paper-input and  input, see
https://jsfiddle.net/mpr5jdhn/. Why does the search highlighting algorithm need
to be modified? Isn't it working as intended?

[Moe, 2016-09-23 18:45:42]

On 2016/09/23 18:29:14, dpapad wrote:
> On 2016/09/23 at 17:18:05, mahmadi wrote:
> > Hi Demetrios, Please take a look at this CL.
> 
> I don't think the reported issue has anything to do with the searching
> algorithm. When you drag and drop text from one input to another, the original
> input becomes empty (happens both for paper-input and  input, see
> https://jsfiddle.net/mpr5jdhn/. Why does the search highlighting algorithm
need
> to be modified? Isn't it working as intended?

I don't think the issue being reported is the input becoming empty. Please note
3rd repro step:

"3. Now enter a new person name, click on back arrow button ***and observe the
person name***"

dragging and dropping the name into the search field is just the tester's way of
searching for the name instead of typing it...

what's happening here is that the profile name on the people card won't get
updated anymore after being partially/fully highlighted. The description of the
CL explains why.

[dpapad, 2016-09-23 19:36:05]

Thanks, I understand now what this CL is fixing, LGTM

Technically, there is still a bug in the solution. If the hidden original text
node changes text value, the obsolete highlighted copy is still displayed. This
can be solved by adding a MutationObserver on the original hidden text node, and
if the text changes, we should remove the highlight and reveal the original
node. I'll file a separate bug for this, not a very high priority IMO.

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/search_settings.js (right):

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_settings.js:10: var
ORG_CONTENT_CSS_CLASS = 'search-highlight-original-content';
Nit: Probably better to stay away from non-established abbreviations. Just
rename to ORIGINAL_CONTENT_CSS_CLASS

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_settings.js:89: // Keep the existing
node around for when the highlights are removed.
For future reference, explain in the comment that the original text node might
be involved in data-binding and therefore should not be discarded.

[Moe, 2016-09-23 23:18:18]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[Moe, 2016-09-23 23:18:24]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 23:18:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Moe, 2016-09-23 23:22:54]

Ah you're right. Although we shouldn't necessarily reveal the original node once
updated. The updated text might still match the search terms. in that case we
need to remove the highlights and redo it. Feel free to file and assign to me.
That's an interesting bug to fix while in i'm LA :)

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
File chrome/browser/resources/settings/search_settings.js (right):

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_settings.js:10: var
ORG_CONTENT_CSS_CLASS = 'search-highlight-original-content';
On 2016/09/23 19:36:05, dpapad wrote:
> Nit: Probably better to stay away from non-established abbreviations. Just
> rename to ORIGINAL_CONTENT_CSS_CLASS

Done.

https://codereview.chromium.org/2360853006/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_settings.js:89: // Keep the existing
node around for when the highlights are removed.
On 2016/09/23 19:36:05, dpapad wrote:
> For future reference, explain in the comment that the original text node might
> be involved in data-binding and therefore should not be discarded.

Done.

[commit-bot: I haz the power, 2016-09-24 00:41:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-24 00:41:26]

Dry run: This issue passed the CQ dry run.

[Moe, 2016-09-26 14:00:14]

The CQ bit was checked by mahmadi@chromium.org

[Moe, 2016-09-26 14:00:14]

The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org
Link to the patchset: https://codereview.chromium.org/2360853006/#ps20001
(title: "Addressed comments")

[commit-bot: I haz the power, 2016-09-26 14:00:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 16:41:08]

Description was changed from

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

Previously, the search highlighting logic got rid of the original text node.
That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

Previously, the search highlighting logic got rid of the original text node.
That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-26 16:41:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-26 16:44:29]

Description was changed from

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

Previously, the search highlighting logic got rid of the original text node.
That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Keeps the original text node when search highlighting happens so it can later be
restored.

Previously, the search highlighting logic got rid of the original text node.
That meant it got rid of polymer bindings too.

BUG=639259
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/ea2290c7cb208a3575d1e2589da2fda9450491ab
Cr-Commit-Position: refs/heads/master@{#420906}
==========

[commit-bot: I haz the power, 2016-09-26 16:44:31]

Patchset 2 (id:??) landed as
https://crrev.com/ea2290c7cb208a3575d1e2589da2fda9450491ab
Cr-Commit-Position: refs/heads/master@{#420906}