Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(378)

Issue 302323004: Prepare Layout tests for ICU upgrade to 52 in (Closed)

Created:
6 years, 6 months ago by jungshik at Google
Modified:
6 years, 6 months ago
Reviewers:
tkent
CC:
blink-reviews
Visibility:
Public.

Description

Prepare Layout tests for ICU upgrade to 52 in A newer version of CLDR/ICU uses ',' as a delimeter between Date and Time in en-US locale leading to the failure of a bunch of date-time-related layout tests on Linux/Android where ICU is used to format date/time. In addition, date time format and month names have changed in several locales, Hindi, Russian, Arabic, and so forth. For most of tests, rebaseline after rolling ICU to 52 would work. They're marked as such in TestExpectations. For the following 3 tests, a bit more changes are required. 1. fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-value-set-empty.html - Add *-expected.txt for Linux (will also be used by Android) with a comma. - Marked as FAILURE in TestExpectation. 2. fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-preserve-value-after-history-back.html - Adjusted the expected result hardcoded in the test depending on whether the formatted field text has a comma or not. - Add *-expected.txt for Linux (will also be used by Android) with a comma. - Marked as FAILURE in TestExpectation. 3. LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html - Adjusted the x-coordinate of the mouse click depending on whether the formated field text has a comma or not so that it can pass regardless of whether a comma is present or not. BUG=378919, 379365, 132145 TEST=datetime* and calendar-picker-related layout tests run as expected (fail or pass). After ICU roll to 52, some would pass unexpectedly. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175442

Patch Set 1 #

Total comments: 1

Messages

Total messages: 5 (0 generated)
jungshik at Google
Can you take a look? Thanks
6 years, 6 months ago (2014-06-02 22:33:31 UTC) #1
tkent
lgtm. It's ok to land as is. https://codereview.chromium.org/302323004/diff/1/LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html File LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html (right): https://codereview.chromium.org/302323004/diff/1/LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html#newcode80 LayoutTests/fast/forms/datetimelocal-multiple-fields/datetimelocal-multiple-fields-mouse-events.html:80: debug('==> Focus ...
6 years, 6 months ago (2014-06-03 01:45:07 UTC) #2
jungshik at Google
The CQ bit was checked by jshin@chromium.org
6 years, 6 months ago (2014-06-03 21:22:43 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jshin@chromium.org/302323004/1
6 years, 6 months ago (2014-06-03 21:23:09 UTC) #4
commit-bot: I haz the power
6 years, 6 months ago (2014-06-04 04:33:09 UTC) #5
Message was sent while issue was closed.
Change committed as 175442

Powered by Google App Engine
This is Rietveld 408576698