New issue
Advanced search Search tips

Issue 780781 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: 2017-11-13
OS: Mac
Pri: 3
Type: Bug-Regression



Sign in to add a comment

[Missing Test]: Label layout issue in Urkainian localisation (Chrome Welcome Dialog)

Project Member Reported by krajshree@chromium.org, Nov 2 2017

Issue description

Automated tests for the below commit have been missing.Please add test coverage ASAP to avoid regressions in future.

CL: 
----
https://chromium.googlesource.com/chromium/src.git/+/f65944765c1e3072bf9ae72fa9f4fafbd31453bb

Ref Bug: 
---------
https://bugs.chromium.org/p/chromium/issues/detail?id=776683

Thank you...!!
 
Labels: -Pri-1 -ReleaseBlock-Beta Pri-3
NextAction: 2017-11-06
Dropping to Pri-3, stripping ReleaseBlock-Beta, and marking for NextAction next week. In principle I can add a unit test for this, but in practice I'm not sure what the value would be. I will have a more detailed look and see how difficult the test would be.
The NextAction date has arrived: 2017-11-06
NextAction: 2017-11-13
Okay. I have somewhat of an idea for how to test this, but it will require a little bit of scaffolding. Here's my rough idea:

1) I need to fix the crashes that --mangle-localized-strings (from <https://chromium-review.googlesource.com/c/chromium/src/+/749582>) exposed. This will be a followup CL.
2) Once that's done, I can write a test that runs the first run flow with --mangle-localized-strings, and then
3) Check whether any of the elements in the UI either overlap other elements that aren't their ancestors, or have bounds outside the content view's bounds.

I think that would prevent any similar regressions. I'll work on this this week; next update next week.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 8 2017

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

commit a452b186ee3b2719bcb9c19f80b7f604b1f1664e
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Wed Nov 08 05:43:00 2017

ui: don't mangle integral constants

Bug:  780781 
Change-Id: I3dd4b57d4017754a2ff736d3fb37d5cc92f9faee
Reviewed-on: https://chromium-review.googlesource.com/753792
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514764}
[modify] https://crrev.com/a452b186ee3b2719bcb9c19f80b7f604b1f1664e/ui/base/resource/resource_bundle.cc

The NextAction date has arrived: 2017-11-13
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 14 2017

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

commit 9e8f6730653ae07ed8f278464c1110d0fdec7d6f
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Nov 14 22:00:10 2017

cocoa: add test coverage for first run layout

This change:
1) Adds ui::ValidateViewTree, which inspects a Cocoa view tree for a variety
   (well, a couple) of correctness problems to do with layout;
2) Adds ResourceBundle::set_mangle_localized_strings() to force string mangling
   on for a specific part of a unit test;
3) Adds a new FirstRunDialogControllerTest that calls ui::ValidateViewTree on
   the first run dialog's content view;
4) Fixes two layout issues in the first run dialog exposed by the mentioned new
   test

Bug:  780781 
Change-Id: If5f8ea92520bbbd5e665a1f5e4a4eb1388a48e76
Reviewed-on: https://chromium-review.googlesource.com/769347
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516446}
[modify] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/chrome/browser/ui/cocoa/first_run_dialog_controller.mm
[modify] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/chrome/browser/ui/cocoa/first_run_dialog_controller_unittest.mm
[modify] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/ui/base/BUILD.gn
[modify] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/ui/base/resource/resource_bundle.h
[add] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/ui/base/test/view_tree_validator.h
[add] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/ui/base/test/view_tree_validator.mm
[add] https://crrev.com/9e8f6730653ae07ed8f278464c1110d0fdec7d6f/ui/base/test/view_tree_validator_unittest.mm

Status: Fixed (was: Assigned)
There is now test coverage for this specific bug, and also for this entire class of bugs on the first run dialog.

Sign in to add a comment