New issue
Advanced search Search tips

Issue 659323 link

Starred by 1 user
Status: Fixed
Owner:
hcarmona@chromium.org
Closed: Oct 2016
Cc:
dbeam@chromium.org
dpa...@chromium.org
steve...@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

[MD Settings] Only first 500 search engines displayed

Project Member Reported by hcarmona@chromium.org, Oct 25 2016 
Similar to  Issue 654738 , affects:
chrome://md-settings/searchEngines

Steps to reproduce:
1. Navigate to chrome://md-settings/searchEngines
2. Have more than 500 search engines
      var seproxy = settings.SearchEnginesBrowserProxyImpl.getInstance();
      for (var i = 1; i <= 1000; ++i) {
        seproxy.searchEngineEditStarted(-1);
        seproxy.searchEngineEditCompleted('_' + i, '_' + i, '_' + i);
      }
3. Scroll past 500

Expected to see more search engines, only seeing 500.
only 500.png
139 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a88d285f8f51ecc9425f1f3947706ed99de07af9

commit a88d285f8f51ecc9425f1f3947706ed99de07af9
Author: hcarmona <hcarmona@chromium.org>
Date: Fri Oct 28 17:54:51 2016

Make search engine iron-list aware of the global scrollTarget.

This allows virtual scrolling which fixes the issue where only 500 items
are displayed in the list.

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

Review-Url: https://codereview.chromium.org/2454483002
Cr-Commit-Position: refs/heads/master@{#428417}

[modify] https://crrev.com/a88d285f8f51ecc9425f1f3947706ed99de07af9/chrome/browser/resources/settings/search_engines_page/compiled_resources2.gyp
[modify] https://crrev.com/a88d285f8f51ecc9425f1f3947706ed99de07af9/chrome/browser/resources/settings/search_engines_page/search_engines_list.html
[modify] https://crrev.com/a88d285f8f51ecc9425f1f3947706ed99de07af9/chrome/browser/resources/settings/search_engines_page/search_engines_list.js

Comment 2 by hcarmona@chromium.org, Oct 28 2016

Status: Fixed (was: Started)

Comment 3 by dpa...@chromium.org, Nov 1 2016

Cc: steve...@chromium.org dbeam@chromium.org
What is the relationship of this fix and the CrScrollableBehavior? Before this CL search_engines_list element was implementing CrScrollableBehavior and was suffering by https://bugs.chromium.org/p/chromium/issues/detail?id=657336. Now it no longer uses that behavior, but I am guessing it somehow gets populated correctly? Is CrScrollableBehavior still relevant?

Comment 4 by hcarmona@chromium.org, Nov 1 2016

Cc: dpa...@chromium.org
CrScrollableBehavior is relevant in most places that we use iron-list in order to fix an issue where the list is initially empty.

GlobalScrollTargetBehavior should be used in places where the scroll target is not the direct parent (i.e. when the iron-list should scroll the height of the page). Setting the scroll target also corrects the initial size issue, so it's not necessary to have CrScrollableBehavior when an element has GlobalScrollTargetBehavior.

Steven and I had a conversation in this CL: http://crrev.com/2448653002
►

Sign in to add a comment