Properly check the visibility of elements in browsertests

Properly check the visibility of elements in browsertests

No change in production logic.

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG=716542
Review-Url: https://codereview.chromium.org/2845423002
Cr-Commit-Position: refs/heads/master@{#468111}
Committed: https://chromium.googlesource.com/chromium/src/+/3d74f72c743bef9d6fb48d06b7638fdf0cb7c5aa

[Nate Fischer, 2017-04-28 17:19:44]

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

[commit-bot: I haz the power, 2017-04-28 17:20:33]

Dry run: CQ is trying da patch.

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

[Nate Fischer, 2017-04-28 17:24:16]

ntfschr@chromium.org changed reviewers:
+ jialiul@chromium.org

[Nate Fischer, 2017-04-28 17:24:17]

PTAL. Another cherry pick from crrev/2848733002. This also does not depend on
the change in that CL, it just got exposed since browser tests ran against the
mobile layout for apparently the first time. This of course is 100% compatible
with the desktop layout too.

[Nate Fischer, 2017-04-28 17:27:53]

Description was changed from

==========
Properly check the visibility of elements

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG=716542
==========

to

==========
Properly check the visibility of elements in browsertests

No change in production logic.

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG=716542
==========

[Jialiu Lin, 2017-04-28 18:14:08]

LGTM  Thanks!

https://codereview.chromium.org/2845423002/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/2845423002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:570: // Don't
check opacity, since the css transition may actually leave
nit: "Don't" -> "Do not"

codereview.chromium.org seems confused on how to properly display syntax colorof
this part beause of this '

[commit-bot: I haz the power, 2017-04-28 18:14:09]

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

[commit-bot: I haz the power, 2017-04-28 18:14:10]

Dry run: This issue passed the CQ dry run.

[Nate Fischer, 2017-04-28 18:26:25]

https://codereview.chromium.org/2845423002/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc (right):

https://codereview.chromium.org/2845423002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc:570: // Don't
check opacity, since the css transition may actually leave
On 2017/04/28 18:14:08, Jialiu Lin wrote:
> nit: "Don't" -> "Do not"
> 
> http://codereview.chromium.org seems confused on how to properly display
syntax colorof
> this part beause of this '

Done.

[Nate Fischer, 2017-04-28 18:27:12]

The CQ bit was checked by ntfschr@chromium.org

[Nate Fischer, 2017-04-28 18:27:13]

The patchset sent to the CQ was uploaded after l-g-t-m from jialiul@chromium.org
Link to the patchset: https://codereview.chromium.org/2845423002/#ps20001
(title: ""don't" -> "do not"")

[commit-bot: I haz the power, 2017-04-28 18:28:31]

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, 2017-04-28 20:26:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493404032674090,
"parent_rev": "94297f25fdf492db4eedd9524c413c7566994567", "commit_rev":
"3d74f72c743bef9d6fb48d06b7638fdf0cb7c5aa"}

[commit-bot: I haz the power, 2017-04-28 20:26:27]

Description was changed from

==========
Properly check the visibility of elements in browsertests

No change in production logic.

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG=716542
==========

to

==========
Properly check the visibility of elements in browsertests

No change in production logic.

Visibility of elements must be checked by recursively checking the
visibility of parent elements, since any element inside an invisible
element will also be invisible.

This issue does not present itself in the Desktop layout, because
.hidden is defined differently (with display: none). Since the mobile
layout defines it with opacity, we need to recursively check parents.

It's not sufficient to check the opacity value itself, because CSS
transitions may not have finished by the time we check. Instead, we
simply check the CSS class and trust it to hide things properly.

BUG=716542

Review-Url: https://codereview.chromium.org/2845423002
Cr-Commit-Position: refs/heads/master@{#468111}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d74f72c743bef9d6fb48d06b763...
==========

[commit-bot: I haz the power, 2017-04-28 20:26:28]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/3d74f72c743bef9d6fb48d06b763...