New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 753119 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

ios_web_shell_egtests ContextMenuTestCase fail on iOS 11, iPad

Project Member Reported by liaoyuke@chromium.org, Aug 7 2017

Issue description

testContextMenuWebkitTouchCalloutNone and testContextMenuWebkitTouchCalloutNoneFromAncestor fail on iOS 11, iPad Air.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 7 2017

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

commit ecd838077ea7d335dc7bf283ba73d7d9af28dc90
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Mon Aug 07 23:34:13 2017

Disable web shell ContextMenuTestCase tests on iOS 11 iPad.

testContextMenuWebkitTouchCalloutNone and
testContextMenuWebkitTouchCalloutNoneFromAncestor fail on iOS 11,
iPad Air.

Bug:  753119 
Change-Id: I0d8ebbe4b4fe964c03024e4394ffb1205d9d41e3
Reviewed-on: https://chromium-review.googlesource.com/604501
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492448}
[modify] https://crrev.com/ecd838077ea7d335dc7bf283ba73d7d9af28dc90/ios/web/shell/test/context_menu_egtest.mm

Labels: -Pri-1 Pri-2
Here is the detailed failure log:

Exception Name: AssertionFailedException
Exception Reason: (((testing::WaitUntilConditionOrTimeout( kWaitForVerificationTimeout, ^{ return verified; }))) is true) failed
Exception Details: The action (Long Press for 0.700000 seconds) on element_id link wasn't verified before timing out.

And the exact place where it fails is inside the |id<GREYAction> WebViewVerifiedActionOnElement(WebState* state, id<GREYAction> action, const std::string& element_id);| while waiting for the |verified| variable to be true:

    GREYAssert(testing::WaitUntilConditionOrTimeout(
                   kWaitForVerificationTimeout,
                   ^{
                     return verified;
                   }),
               verification_timeout_message);

This test fails constantly, and doesn't look like a test framework issue.
Components: -Test>iOS Mobile>WebView>Glue Tests>Disabled
Owner: eugene...@chromium.org
Flipping to Eugene to take a look at glue's side.
Components: -Mobile>WebView>Glue
Owner: liaoyuke@chromium.org
But Context Menu works fine on iOS 11, so it does look like an issue with testing framework.
Cc: marq@chromium.org
Labels: -Pri-2 Pri-3
Yes, context menu works on iOS 11, but this test doesn't bring up the context menu at all:

  [[EarlGrey selectElementWithMatcher:web::WebView()]
      performAction:web::LongPressElementForContextMenu(
                        linkID, false /* menu shouldn't appear */)];

I have a very hard time understanding what this test is trying to do. Adding Mark, who is the author of this test, do you have any insights?
I also believe this is blocking me from landing these context menu changes: https://chromium-review.googlesource.com/c/579585

With that change, the same failure occurs with iOS 10 simulator as well. Should I disable these 2 test always in order to land that CL?

It looks like we install javascript to make sure that the element had a mousedown event occur on it. Is that necessary?
I couldn't reproduce the failure on the WKWebView product that is used to report bugs, so I'm not really sure how to fix it. Additionally, whether mousedown event occurs or not is a pretty low level detail that has no direct impact on users, so the expectation  doesn't seem to fit well into an UI test.

Maybe just update the test to assert that no context menu is shown?
If the problem is not reproducible with stock WKWebView, then it can be an issue with our EG framework scripts. I think asserting that context menu is not shown is not quite the same thing (if the test will stop sending touch events and the browser will start ignoring --webkit-Touch-callout-none because of a bug, the test will keep passing).
Cc: michaeldo@chromium.org
I see, that makes sense! So what I can do is that I can try to reproduce this issue on EarlGrey's sample app and file a bug with them. michael said yesterday that he might make some changes to this test to unblock his CL, michael, could you please elaborate a little bit more?
Project Member

Comment 10 by bugdroid1@chromium.org, Aug 21 2017

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

commit ac6388a966a01e925aef6bedd173e7c3e6cbbc91
Author: Yuke Liao <liaoyuke@chromium.org>
Date: Mon Aug 21 19:00:03 2017

Add unit tests for webkit touch callout.

This CL adds unit tests to test the behaviors of -webkit-touch-callout
CSS property.

Bug:  753119 
Change-Id: Id76edb297d38a38592459c7d64a5b3b5fa9eb4f9
Reviewed-on: https://chromium-review.googlesource.com/622951
Commit-Queue: Yuke Liao <liaoyuke@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496011}
[modify] https://crrev.com/ac6388a966a01e925aef6bedd173e7c3e6cbbc91/ios/web/web_state/js/context_menu_js_unittest.mm

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 22 2017

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

commit dbe6942534710f67dfdbd7f2aaa6294292707b2d
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Tue Aug 22 19:20:58 2017

Remove WebkitTouchCallout equals none context menu tests.

These tests fail on iPads running iOS 11 and on the the newly refactored
context menu presentation code in crrev.com/c/579585.

Unittests have already been added to test the behaviors of context menu
elements which set WebkitTouchCallout to none.

Bug:  753119 
Change-Id: I229d7453a14f1d287653848f6d5c85df85b33f6f
Reviewed-on: https://chromium-review.googlesource.com/625061
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496397}
[modify] https://crrev.com/dbe6942534710f67dfdbd7f2aaa6294292707b2d/ios/web/shell/test/context_menu_egtest.mm
[modify] https://crrev.com/dbe6942534710f67dfdbd7f2aaa6294292707b2d/ios/web/shell/test/earl_grey/shell_actions.h
[modify] https://crrev.com/dbe6942534710f67dfdbd7f2aaa6294292707b2d/ios/web/shell/test/earl_grey/shell_actions.mm

Status: Fixed (was: Assigned)
Unittests were added and these eg tests have been removed.
Components: Tests>Disabled
Labels: Test-Disabled

Sign in to add a comment