MD Settings: Remove iron-pages usage from <settings-sync-page>.

MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome and after sync prefs have been fetched, search
for "Please wait". There should be no search hit in the "People" section.
Previously a search hit occurred within a hidden message that was not applicable
to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/9474e1a879494292768c724190da09fd77b64a06
Cr-Commit-Position: refs/heads/master@{#411514}

[dpapad, 2016-08-11 00:46:02]

Description was changed from

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome, search for "Please wait". There should be no
search hit in the "People" section. Previously a search hit occurred within a
hidden message that was not applicable to the current state of the user.
==========

to

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome, search for "Please wait". There should be no
search hit in the "People" section. Previously a search hit occurred within a
hidden message that was not applicable to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-08-11 01:08:52]

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

[commit-bot: I haz the power, 2016-08-11 01:09:48]

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

[dpapad, 2016-08-11 01:17:39]

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

[commit-bot: I haz the power, 2016-08-11 01:17:52]

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

[dpapad, 2016-08-11 01:18:20]

Description was changed from

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome, search for "Please wait". There should be no
search hit in the "People" section. Previously a search hit occurred within a
hidden message that was not applicable to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome and after sync prefs have been fetched, search
for "Please wait". There should be no search hit in the "People" section.
Previously a search hit occurred within a hidden message that was not applicable
to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[dpapad, 2016-08-11 01:25:19]

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

[dpapad, 2016-08-11 01:25:20]

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:127: //if
(settings.getCurrentRoute() == settings.Route.SYNC)
Tommy, I commented out lines 127 and line 161 because otherwise the sync page is
very unfriendly to searching. It displays different contents when
force-rendered, and different when navigated to by a user. 

Do you think removing those lines will cause any problems?

[dpapad, 2016-08-11 01:28:28]

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.html (right):

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.html:35:
hidden$="[[!isStatus_(pages.CONFIGURE, pageStatus_)]]">
FYI, the diff looks pretty misleading after this point. The only real change is
an indentation decrease, because I removed the surrounding <iron-pages> element.

[commit-bot: I haz the power, 2016-08-11 04:00:13]

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

[commit-bot: I haz the power, 2016-08-11 04:00:14]

Dry run: This issue passed the CQ dry run.

[dpapad, 2016-08-11 21:59:14]

Friendly ping. @Tommy see question in previous message.

[tommycli, 2016-08-11 22:14:29]

dpapad: sorry for lag

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:64: * The curernt
page status.
nit: typo

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:127: //if
(settings.getCurrentRoute() == settings.Route.SYNC)
On 2016/08/11 01:25:20, dpapad wrote:
> Tommy, I commented out lines 127 and line 161 because otherwise the sync page
is
> very unfriendly to searching. It displays different contents when
> force-rendered, and different when navigated to by a user. 
> 
> Do you think removing those lines will cause any problems?

Fuuu I think that would be bad... I think I need to rework how this works before
it will play nice with search.

[dpapad, 2016-08-11 23:00:49]

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:64: * The curernt
page status.
On 2016/08/11 at 22:14:28, tommycli wrote:
> nit: typo

Fixed.

https://codereview.chromium.org/2230833003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:127: //if
(settings.getCurrentRoute() == settings.Route.SYNC)
On 2016/08/11 at 22:14:28, tommycli wrote:
> On 2016/08/11 01:25:20, dpapad wrote:
> > Tommy, I commented out lines 127 and line 161 because otherwise the sync
page is
> > very unfriendly to searching. It displays different contents when
> > force-rendered, and different when navigated to by a user. 
> > 
> > Do you think removing those lines will cause any problems?
> 
> Fuuu I think that would be bad... I think I need to rework how this works
before it will play nice with search.

Per our discussion, when the didNavigateToSyncPage() is called the C++ side of
things is being put into a "setup" mode, which is undesirable when the user has
not visited the page manually. So instead of commenting those lines out and
invoking "setup" mode from a forced-rendering, changed the default PageStatus to
CONFIGURE instead. This results in reasonable behavior for the most frequent
case (when no timeout occurred).

[tommycli, 2016-08-11 23:03:32]

lgtm thanks! nit:

https://codereview.chromium.org/2230833003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2230833003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:155: isStatus_:
function(pageStatus) {
err maybe make this argument name expectedPageStatus or something to
disambiguate.

[dpapad, 2016-08-12 00:52:47]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-08-12 00:53:18]

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

[dpapad, 2016-08-12 00:58:43]

https://codereview.chromium.org/2230833003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/sync_page.js (right):

https://codereview.chromium.org/2230833003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/sync_page.js:155: isStatus_:
function(pageStatus) {
On 2016/08/11 at 23:03:32, tommycli wrote:
> err maybe make this argument name expectedPageStatus or something to
disambiguate.

Done.

[dpapad, 2016-08-12 00:58:58]

The CQ bit was unchecked by dpapad@chromium.org

[dpapad, 2016-08-12 00:59:32]

The CQ bit was checked by dpapad@chromium.org

[dpapad, 2016-08-12 00:59:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2230833003/#ps80001
(title: "nit")

[commit-bot: I haz the power, 2016-08-12 00:59:49]

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-08-12 01:59:29]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-12 02:01:19]

Description was changed from

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome and after sync prefs have been fetched, search
for "Please wait". There should be no search hit in the "People" section.
Previously a search hit occurred within a hidden message that was not applicable
to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: Remove iron-pages usage from <settings-sync-page>.

This facilitates the searching algorithm to ignore hidden nodes.
<iron-pages> hides the currently not displayed pages using a custom CSS
:host > ::content > :not(.iron-selected) { display: none !important; }
which bypasses the searching algorithm's visibility checks. Using the
"hidden" attribute instead of <iron-pages> fixes the issue.

BUG=608535
TEST=When logged in to chrome and after sync prefs have been fetched, search
for "Please wait". There should be no search hit in the "People" section.
Previously a search hit occurred within a hidden message that was not applicable
to the current state of the user.
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/9474e1a879494292768c724190da09fd77b64a06
Cr-Commit-Position: refs/heads/master@{#411514}
==========

[commit-bot: I haz the power, 2016-08-12 02:01:20]

Patchset 5 (id:??) landed as
https://crrev.com/9474e1a879494292768c724190da09fd77b64a06
Cr-Commit-Position: refs/heads/master@{#411514}