New issue
Advanced search Search tips

Issue 881859 link

Starred by 1 user
Status: Verified
Owner:
nicolaso@chromium.org
Closed: Oct 26
Cc:
mpear...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Unit test depends on specific national keyboard layout

Project Member Reported by yn...@vivaldi.com, Sep 7 
Chrome Version: v70
OS: Win 7, Win 10

The interactive_ui_tests test OmniboxViewTest.SearchDisabledDontCrashOnQuestionMark fails with

  chrome/browser/ui/omnibox/omnibox_view_browsertest.cc(1035): error: Expected equality of these values:
  ASCIIToUTF16("?")
    Which is: L"?"
  omnibox_view->GetText()
    Which is: L"*"

this test sends the following key code:
 
   SendKey(ui::VKEY_OEM_2, ui::EF_SHIFT_DOWN)
 
According to the definitions VKEY_OEM_2=VK_OEM_2 and according to WinUser.h VK_OEM_2 is defined as
 
   #define VK_OEM_2          0xBF   // '/?' for US
 
However, on other keyboards this character can be something different, e.g. on Norwegian keyboards this key is "'" and "*", and our automatic testers are running in an Icelandic environment, which seems to have the same key assignments.

BTW, the variable "search_keyword" in that test seems unused.

For reference, we have seen a number of such locale specific tests that assume  a US Pacific locale (e.g https://bugs.chromium.org/p/chromium/issues/detail?id=842460 )

Comment 1 by mpear...@chromium.org, Sep 7

Components: Tests>Fails UI>Internationalization
This looks "triaged".  nicolaso@, are you the right owner?  If so, mark it as assigned.  If not, mark it as available.

reporter: you're also invited to fix the test.

I wonder we can simply send the "?" character directly in the text, not shift-2.

Comment 2 by nicolaso@chromium.org, Sep 7

Status: Assigned (was: Untriaged)
Marking as assigned.

I'll look into a way to send characters instead of keycodes.

Comment 3 by nicolaso@chromium.org, Sep 7 

yngve, quick question: is OmniboxViewTest.AcceptKeywordByTypingQuestionMark [1] also failing in your environment? It looks very similar to SearchDisabledDontCrashOnQuestionMark.

[1] https://cs.chromium.org/chromium/src/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc?l=997&rcl=821ebe6c946b5300ce259fc2211f120289060bc7

Comment 4 by yn...@vivaldi.com, Sep 7 

That one has been disabled since early 2017 (that was done by a colleague) AFAICT, probably due to the same problem).

For reference, don't remember what was the reason, but we also disabled OmniboxViewTest.DeleteItem and OmniboxViewTest.SelectAllStaysAfterUpdate a year ago (the comment on that part of the list suggests that they were disabled when integrating v61, and also failed in a local chromium build; others were disabled then, but have been re-enabled since, when we did a re-enable test).
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 10 

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

commit 687bee3c1f44ac989a791a83465e6f19843055a9
Author: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Date: Mon Sep 10 17:43:52 2018

Fix an Omnibox test on non-US keyboard layouts

Bug:  881859 
Change-Id: Ic6d6945726d1dfd4fda5b59031c785d321730a09
Reviewed-on: https://chromium-review.googlesource.com/1214243
Reviewed-by: Tommy Li <tommycli@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589967}
[modify] https://crrev.com/687bee3c1f44ac989a791a83465e6f19843055a9/chrome/browser/ui/omnibox/omnibox_view_browsertest.cc

Comment 7 by nicolaso@chromium.org, Oct 26

Status: Fixed (was: Assigned)
Marking as fixed, feel free to re-open if this is still an issue.

yngve, you can mark it as 'verified' if the fix does work.

Comment 8 by yn...@vivaldi.com, Oct 26

Status: Verified (was: Fixed)
This seems to work OK now in my v71 based build. Thanks
►

Sign in to add a comment