Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2120353003:
Fix TouchSelectionControllerImpl tests failing with DPI=125% (Closed)
|
Created:
4 years, 5 months ago by Tomasz Moniuszko Modified:
4 years, 5 months ago Reviewers:
mohsen CC:
chromium-reviews, tfarina Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix TouchSelectionControllerImpl tests failing with DPI=125%
Make textfield a bit larger so text fits it when system DPI is set to 125%.
Fixes following tests:
TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText
TouchSelectionControllerImplTest.SelectRectCallbackTest
TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest
TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest
TouchSelectionControllerImplTest.SelectionInTextfieldTest
BUG=626264
Committed: https://crrev.com/0bb7f67eec734651731263d35879d5a93b9e3bad
Cr-Commit-Position: refs/heads/master@{#405743}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 24 (9 generated)
tmoniuszko@opera.com changed reviewers: + mohsen@chromium.org
Can you tell me how can I set DPI to 125% to run the tests and understand what's happening here?
On 2016/07/06 16:44:36, mohsen wrote: > Can you tell me how can I set DPI to 125% to run the tests and understand what's > happening here? Sure. I created https://crbug.com/626264 with detailed bug description.
Description was changed from ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG= ========== to ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG=626264 ==========
Thanks for creating the bug report! https://codereview.chromium.org/2120353003/diff/1/ui/views/touchui/touch_sele... File ui/views/touchui/touch_selection_controller_impl_unittest.cc (right): https://codereview.chromium.org/2120353003/diff/1/ui/views/touchui/touch_sele... ui/views/touchui/touch_selection_controller_impl_unittest.cc:100: textfield_->SetBoundsRect(gfx::Rect(0, 0, 200, 21)); Is this one extra pixel going to be enough for all possible DPI scales?
On 2016/07/07 19:41:13, mohsen wrote: > Thanks for creating the bug report! > > https://codereview.chromium.org/2120353003/diff/1/ui/views/touchui/touch_sele... > File ui/views/touchui/touch_selection_controller_impl_unittest.cc (right): > > https://codereview.chromium.org/2120353003/diff/1/ui/views/touchui/touch_sele... > ui/views/touchui/touch_selection_controller_impl_unittest.cc:100: > textfield_->SetBoundsRect(gfx::Rect(0, 0, 200, 21)); > Is this one extra pixel going to be enough for all possible DPI scales? I haven't tested all possible custom settings but only predefined and commonly used ones: 100%, 125%, 150%, 200%, 250%. One pixel is enough in all these cases.
lgtm
The CQ bit was checked by tmoniuszko@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/07/12 10:31:09, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) I'm not sure why exception happed. I think I'll just retry.
The CQ bit was checked by tmoniuszko@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by tmoniuszko@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG=626264 ========== to ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG=626264 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG=626264 ========== to ========== Fix TouchSelectionControllerImpl tests failing with DPI=125% Make textfield a bit larger so text fits it when system DPI is set to 125%. Fixes following tests: TouchSelectionControllerImplTest.DoubleTapInTextfieldWithCursorHandleShouldSelectText TouchSelectionControllerImplTest.SelectRectCallbackTest TouchSelectionControllerImplTest.SelectRectInBidiCallbackTest TouchSelectionControllerImplTest.SelectionInBidiTextfieldTest TouchSelectionControllerImplTest.SelectionInTextfieldTest BUG=626264 Committed: https://crrev.com/0bb7f67eec734651731263d35879d5a93b9e3bad Cr-Commit-Position: refs/heads/master@{#405743} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/0bb7f67eec734651731263d35879d5a93b9e3bad Cr-Commit-Position: refs/heads/master@{#405743} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
