Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2454023002:
MD Settings: Fix <select> search highlighting. (Closed)
|
Created:
4 years, 1 month ago by dpapad Modified:
4 years, 1 month ago Reviewers:
Dan Beam CC:
arv+watch_chromium.org, chromium-reviews, dbeam+watch-settings_chromium.org, michaelpg+watch-md-settings_chromium.org, michaelpg+watch-md-ui_chromium.org, stevenjb+watch-md-settings_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMD Settings: Fix <select> search highlighting.
<select> elements should not be highlighted with yellow rectangles
because the highlight CSS style is ignored anyway and breaks the
<select> (duplicate strings displayed). Just including the parent
<settings-section> should be sufficient for now (matches old Options).
Also add the first unit tests for the DOM searching algorithm internals.
BUG=659617
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/9128a695732eba4729713506cf40268e1aada67a
Cr-Commit-Position: refs/heads/master@{#429122}
Patch Set 1 : Revise. #
Total comments: 1
Patch Set 2 : Nt. #
Total comments: 3
Patch Set 3 : Separate test fixture #Patch Set 4 : Fix browserPreload path. #
Total comments: 13
Patch Set 5 : Address comments. #
Total comments: 19
Patch Set 6 : More nits. #Patch Set 7 : Pong #Patch Set 8 : Rebase #
Total comments: 3
Patch Set 9 : Attempting bot fix. #
Total comments: 2
Patch Set 10 : Use loadScript. #
Messages
Total messages: 55 (26 generated)
Description was changed from ========== MD Settings: Fix <select> search highlighting. Also add the first unit test for the searching algorithm. BUG=659617 ========== to ========== MD Settings: Fix <select> search highlighting. Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== MD Settings: Fix <select> search highlighting. Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by dpapad@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Just including the parent <settings-section> should be sufficient. Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Just including the parent <settings-section> should be sufficient. Also add the first unit test for the searching algorithm. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Just including the parent <settings-section> should be sufficient for now (matches old Options). Also add the first unit tests for the DOM searching algorithm internals. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Patchset #3 (id:40001) has been deleted
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
dpapad@chromium.org changed reviewers: + dbeam@chromium.org
Tests! (Expecting green results on patch 1). https://codereview.chromium.org/2454023002/diff/60001/chrome/browser/resource... File chrome/browser/resources/settings/search_settings.js (right): https://codereview.chromium.org/2454023002/diff/60001/chrome/browser/resource... chrome/browser/resources/settings/search_settings.js:154: // displayed. Just revealing the section is sufficient. Highlighting <select> elements with a search bubble instead of yellow rectangles might be a nice improvement over the previous Options UI. We can file a bug and prioritize separately though (prefer not to do it in this CL). https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... chrome/test/data/webui/settings/search_settings_test.js:32: assertFalse(section.hiddenBySearch); I believe this is the assertion you were talking about earlier over chat.
https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... chrome/test/data/webui/settings/cr_settings_browsertest.js:894: 'search_settings_test.js', can you make a different @constructor for this file instead of combining with CrSettingsMainPageTest?
https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/80001/chrome/test/data/webui/... chrome/test/data/webui/settings/cr_settings_browsertest.js:894: 'search_settings_test.js', On 2016/10/27 at 01:29:06, Dan Beam wrote: > can you make a different @constructor for this file instead of combining with CrSettingsMainPageTest? Done. I was also able to trim the deps needed to load for this test (browserPreload now points to settings_section.html).
The CQ bit was checked by dpapad@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:13: /* /** https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:19: var htmlBefore = document.body.innerHTML = why are you changing document.body's innerHTML rather than making a new element and changing its innerHTML? https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:20: `<settings-section hidden-by-search> why are you adding hidden-by-search and checking it rather than just checking assertFalse(section.hiddenbySearch) after searching? https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:24: document.body.innerHTML = htmlBefore; what is all this assignment devilry? https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:56: /* same https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:62: PolymerTest.clearBody(); why do this if it's already done in setup? https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:79: var highlighWrapper = select.querySelector( highlightWrapper
https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:13: /* On 2016/10/27 at 21:24:53, Dan Beam wrote: > /** Done. Technically this is only needed if you have @ annotations, AFAIU. https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:19: var htmlBefore = document.body.innerHTML = On 2016/10/27 at 21:24:53, Dan Beam wrote: > why are you changing document.body's innerHTML rather than making a new element and changing its innerHTML? Removed. It was an accidental leftover of a previous version of the code. https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:20: `<settings-section hidden-by-search> On 2016/10/27 at 21:24:53, Dan Beam wrote: > why are you adding hidden-by-search and checking it rather than just checking assertFalse(section.hiddenbySearch) after searching? Well, for two reasons. a) If I don't add it, then how can I be sure that it was added and removed VS never added? Pre-emptively adding it ensures that it is in fact removed by the code being tested. b) assertTrue(section.hiddenBySearch), ensures that settings_section.html was actually loaded as a Polymer element. In a previous patch, I accidentally used a wrong browserPreload path (settings_main/settings_section.html), and the test still passed (because I was only checking for assertFalse(section.hiddenBySearch), which passed on an unknown to the browser <settings-section> element (no shadow DOM). https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:24: document.body.innerHTML = htmlBefore; On 2016/10/27 at 21:24:53, Dan Beam wrote: > what is all this assignment devilry? Fixed. https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:56: /* On 2016/10/27 at 21:24:53, Dan Beam wrote: > same Done. https://codereview.chromium.org/2454023002/diff/120001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:62: PolymerTest.clearBody(); On 2016/10/27 at 21:24:53, Dan Beam wrote: > why do this if it's already done in setup? Removed.
https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... File chrome/browser/resources/settings/search_settings.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... chrome/browser/resources/settings/search_settings.js:156: if (node.parentNode.nodeName != 'OPTION') wait, why can't we just point an arrow at the <select>?
On 2016/10/27 at 21:37:15, dbeam wrote: > https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... > File chrome/browser/resources/settings/search_settings.js (right): > > https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... > chrome/browser/resources/settings/search_settings.js:156: if (node.parentNode.nodeName != 'OPTION') > wait, why can't we just point an arrow at the <select>? Already commented on this. See https://codereview.chromium.org/2454023002#msg11
https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... File chrome/browser/resources/settings/search_settings.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... chrome/browser/resources/settings/search_settings.js:156: if (node.parentNode.nodeName != 'OPTION') TODO(dpapad): highlight <select> controls even if we can't highlight the <option>s. https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/cr_settings_browsertest.js:925: '../../../../browser/resources/settings/search_settings.js', ROOT_PATH + 'browser/resources/settings/search_settings.js', https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:18: var textBefore = 'FooSettingsFoo'; textBefore -> optionText? https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:23: </settings-section>`; indent off? https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:26: var div = section.queryEffectiveChildren('#mydiv'); why can't this just be $('mydiv')? https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:32: var highlighWrapper = div.querySelector('.search-highlight-wrapper'); highlightWrapper https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:53: remove \n https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:63: <select> indent off?
https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... File chrome/browser/resources/settings/search_settings.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/browser/resourc... chrome/browser/resources/settings/search_settings.js:156: if (node.parentNode.nodeName != 'OPTION') On 2016/10/27 at 21:46:09, Dan Beam wrote: > TODO(dpapad): highlight <select> controls even if we can't highlight the <option>s. Added. https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/cr_settings_browsertest.js:925: '../../../../browser/resources/settings/search_settings.js', On 2016/10/27 at 21:46:09, Dan Beam wrote: > ROOT_PATH + 'browser/resources/settings/search_settings.js', Done. https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:23: </settings-section>`; On 2016/10/27 at 21:46:09, Dan Beam wrote: > indent off? Done, although I think this is arguable. Is it off because you are considering it as HTML? Or are you considering as any JS string? var foo = `<a> <b></b> </a>`; VS var foo = `This is a very loooooooong string. Is the indent off?` https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:26: var div = section.queryEffectiveChildren('#mydiv'); On 2016/10/27 at 21:46:09, Dan Beam wrote: > why can't this just be $('mydiv')? Changed to document.querySelector. Not using "$" because ReferenceError: $ is not defined at Context.<anonymous> (search_settings_test.js:27:28) https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:32: var highlighWrapper = div.querySelector('.search-highlight-wrapper'); On 2016/10/27 at 21:46:09, Dan Beam wrote: > highlightWrapper Done. https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:53: On 2016/10/27 at 21:46:09, Dan Beam wrote: > remove \n OK. https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:63: <select> On 2016/10/27 at 21:46:09, Dan Beam wrote: > indent off? OK.
https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:18: var textBefore = 'FooSettingsFoo'; On 2016/10/27 21:46:09, Dan Beam wrote: > textBefore -> optionText? ping
https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:18: var textBefore = 'FooSettingsFoo'; On 2016/10/27 at 22:15:52, Dan Beam wrote: > On 2016/10/27 21:46:09, Dan Beam wrote: > > textBefore -> optionText? > > ping Done.
On 2016/10/27 at 22:24:51, dpapad wrote: > https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... > File chrome/test/data/webui/settings/search_settings_test.js (right): > > https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... > chrome/test/data/webui/settings/search_settings_test.js:18: var textBefore = 'FooSettingsFoo'; > On 2016/10/27 at 22:15:52, Dan Beam wrote: > > On 2016/10/27 21:46:09, Dan Beam wrote: > > > textBefore -> optionText? > > > > ping > > Done. Friendly 24h ping.
lgtm https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... File chrome/test/data/webui/settings/search_settings_test.js (right): https://codereview.chromium.org/2454023002/diff/140001/chrome/test/data/webui... chrome/test/data/webui/settings/search_settings_test.js:18: var textBefore = 'FooSettingsFoo'; On 2016/10/27 22:24:50, dpapad wrote: > On 2016/10/27 at 22:15:52, Dan Beam wrote: > > On 2016/10/27 21:46:09, Dan Beam wrote: > > > textBefore -> optionText? > > > > ping > > Done. thx
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
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/...)
The CQ bit was checked by dpapad@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dpapad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2454023002/#ps200001 (title: "Rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... chrome/test/data/webui/settings/cr_settings_browsertest.js:925: ROOT_PATH + 'chrome/browser/resources/settings/search_settings.js', Even though this works locally, it fails on the bots. Have not figured out why the bots can't find this file (hitting https://cs.chromium.org/chromium/src/chrome/test/base/javascript_browser_test...). Any help is appreciated.
https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... chrome/test/data/webui/settings/cr_settings_browsertest.js:925: ROOT_PATH + 'chrome/browser/resources/settings/search_settings.js', On 2016/11/01 01:45:40, dpapad wrote: > Even though this works locally, it fails on the bots. Have not figured out why > the bots can't find this file (hitting > https://cs.chromium.org/chromium/src/chrome/test/base/javascript_browser_test...). > Any help is appreciated. trying doing something like: suiteSetup(function() { return this.importHtml('.../search_settings.html'); }); in search_settings.js (as your browsePreload doesn't depend on search_settings.js directly)
https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... File chrome/test/data/webui/settings/cr_settings_browsertest.js (right): https://codereview.chromium.org/2454023002/diff/200001/chrome/test/data/webui... chrome/test/data/webui/settings/cr_settings_browsertest.js:925: ROOT_PATH + 'chrome/browser/resources/settings/search_settings.js', > trying doing something like: > > suiteSetup(function() { > return this.importHtml('.../search_settings.html'); > }); > > in search_settings.js (as your browsePreload doesn't depend on search_settings.js directly) Added chrome/browser/resources/settings to the appropriate list in chrome/test/BUILD.gn which makes all settings src files available to bots (similar to ui/webui/resources/js). Let me know if you are OK with this fix, instead of adding a new dummy search_settings.html file.
https://codereview.chromium.org/2454023002/diff/220001/chrome/test/BUILD.gn File chrome/test/BUILD.gn (right): https://codereview.chromium.org/2454023002/diff/220001/chrome/test/BUILD.gn#n... chrome/test/BUILD.gn:1937: "//chrome/browser/resources/settings/", this is a lot of files. i think 1 import is better. you can also use loadScript() if you really really hate the idea of ... a 1 line html file
https://codereview.chromium.org/2454023002/diff/220001/chrome/test/BUILD.gn File chrome/test/BUILD.gn (right): https://codereview.chromium.org/2454023002/diff/220001/chrome/test/BUILD.gn#n... chrome/test/BUILD.gn:1937: "//chrome/browser/resources/settings/", On 2016/11/01 at 19:33:54, Dan Beam wrote: > this is a lot of files. i think 1 import is better. you can also use loadScript() if you really really hate the idea of ... a 1 line html file Done.
The CQ bit was checked by dpapad@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2454023002/#ps240001 (title: "Use loadScript.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #10 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Just including the parent <settings-section> should be sufficient for now (matches old Options). Also add the first unit tests for the DOM searching algorithm internals. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== MD Settings: Fix <select> search highlighting. <select> elements should not be highlighted with yellow rectangles because the highlight CSS style is ignored anyway and breaks the <select> (duplicate strings displayed). Just including the parent <settings-section> should be sufficient for now (matches old Options). Also add the first unit tests for the DOM searching algorithm internals. BUG=659617 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/9128a695732eba4729713506cf40268e1aada67a Cr-Commit-Position: refs/heads/master@{#429122} ==========
Message was sent while issue was closed.
Patchset 10 (id:??) landed as https://crrev.com/9128a695732eba4729713506cf40268e1aada67a Cr-Commit-Position: refs/heads/master@{#429122} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
