Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(35)

Issue 2593323002: Use the physical-pixel space for native IME on linux platform. (Closed)

Created:
2 years ago by Shu Chen
Modified:
1 year, 11 months ago
CC:
chromium-reviews, yusukes+watch_chromium.org, nektar+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, dmazzoni+watch_chromium.org, aboxhall+watch_chromium.org, jam, jbauman+watch_chromium.org, nona+watch_chromium.org, je_julie, darin-cc_chromium.org, kalyank, yuzo+watch_chromium.org, oshima+watch_chromium.org, piman+watch_chromium.org, elijahtaylor+arcwatch_chromium.org, tfarina, shuchen+watch_chromium.org, dtseng+watch_chromium.org, danakj+watch_chromium.org, James Su, davemoore+watch_chromium.org, yukawa
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Use the physical-pixel space for native IME on linux platform. What's more, this cl makes a clear spec for the coordinates system in ui::TextInputClient. BUG=475718, 360334 Review-Url: https://codereview.chromium.org/2593323002 Cr-Commit-Position: refs/heads/master@{#451980} Committed: https://chromium.googlesource.com/chromium/src/+/da4307a16fe8c0bbecc8158e21ffef8190e23623

Patch Set 1 #

Total comments: 11

Patch Set 2 : refactor the cl, removed the GetDeviceScaleFactor method. #

Total comments: 2

Patch Set 3 : Use the physical-pixel space for native IME on linux platform. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -22 lines) Patch
M chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc View 1 2 3 chunks +13 lines, -4 lines 0 comments Download
M ui/base/ime/text_input_client.h View 1 2 chunks +7 lines, -18 lines 0 comments Download

Messages

Total messages: 55 (32 generated)
Shu Chen
Guys, can you please review this cl? Thanks! msw@: chrome/browser/ui/views/ime_driver/... ui/views/... elijahtaylor@: components/arc/ime/... yukawa@: ui/base/ime/... ...
2 years ago (2016-12-22 01:12:24 UTC) #4
sadrul
https://codereview.chromium.org/2593323002/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc File content/browser/renderer_host/render_widget_host_view_aura.cc (right): https://codereview.chromium.org/2593323002/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc#newcode1293 content/browser/renderer_host/render_widget_host_view_aura.cc:1293: ->GetDisplayNearestWindow(window_).device_scale_factor(); just return |device_scale_factor_|? https://codereview.chromium.org/2593323002/diff/1/ui/base/ime/input_method_auralinux.cc File ui/base/ime/input_method_auralinux.cc (right): https://codereview.chromium.org/2593323002/diff/1/ui/base/ime/input_method_auralinux.cc#newcode285 ...
2 years ago (2016-12-22 01:30:22 UTC) #5
Shu Chen
https://codereview.chromium.org/2593323002/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc File content/browser/renderer_host/render_widget_host_view_aura.cc (right): https://codereview.chromium.org/2593323002/diff/1/content/browser/renderer_host/render_widget_host_view_aura.cc#newcode1293 content/browser/renderer_host/render_widget_host_view_aura.cc:1293: ->GetDisplayNearestWindow(window_).device_scale_factor(); On 2016/12/22 01:30:22, sadrul wrote: > just return ...
2 years ago (2016-12-22 03:02:27 UTC) #8
Shu Chen
Updated the reviews after the cl refactoring.
2 years ago (2016-12-22 03:04:16 UTC) #13
sadrul
lgtm https://codereview.chromium.org/2593323002/diff/20001/chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc File chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc (right): https://codereview.chromium.org/2593323002/diff/20001/chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc#newcode118 chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc:118: rect, views::LinuxUI::instance()->GetDeviceScaleFactor()); Consider using gfx::ConvertRectToPixel (https://cs.chromium.org/chromium/src/ui/gfx/geometry/dip_util.h?dr=CSs&q=ConvertRectToPixel&sq=package:chromium&l=35 ... sorry, ...
2 years ago (2016-12-22 03:25:54 UTC) #14
sadrul
Oh, please make sure to update the CL description too.
2 years ago (2016-12-22 03:26:23 UTC) #15
Shu Chen
On 2016/12/22 03:26:23, sadrul wrote: > Oh, please make sure to update the CL description ...
2 years ago (2016-12-22 03:28:59 UTC) #17
Shu Chen
https://codereview.chromium.org/2593323002/diff/20001/chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc File chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc (right): https://codereview.chromium.org/2593323002/diff/20001/chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc#newcode118 chrome/browser/ui/libgtkui/x11_input_method_context_impl_gtk.cc:118: rect, views::LinuxUI::instance()->GetDeviceScaleFactor()); On 2016/12/22 03:25:53, sadrul (OOO) wrote: > ...
1 year, 11 months ago (2017-02-20 07:55:44 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-20 07:56:19 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/368369)
1 year, 11 months ago (2017-02-20 08:03:51 UTC) #25
Shu Chen
+erg@ for rubber stamp.
1 year, 11 months ago (2017-02-20 08:06:44 UTC) #27
Shu Chen
+estade@ for OWNERS rubber stamp for x11_input_method_context_impl_gtk.cc.
1 year, 11 months ago (2017-02-21 01:26:16 UTC) #33
Elliot Glaysher
lgtm
1 year, 11 months ago (2017-02-21 18:22:13 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-22 00:38:30 UTC) #36
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on ...
1 year, 11 months ago (2017-02-22 02:41:01 UTC) #38
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-22 02:42:37 UTC) #40
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
1 year, 11 months ago (2017-02-22 03:28:27 UTC) #42
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-22 03:36:20 UTC) #44
commit-bot: I haz the power
Exceeded global retry quota
1 year, 11 months ago (2017-02-22 04:45:10 UTC) #46
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-22 05:22:22 UTC) #48
commit-bot: I haz the power
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_rel_ng/builds/394908)
1 year, 11 months ago (2017-02-22 07:30:58 UTC) #50
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2593323002/40001
1 year, 11 months ago (2017-02-22 08:07:22 UTC) #52
commit-bot: I haz the power
1 year, 11 months ago (2017-02-22 10:39:38 UTC) #55
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/da4307a16fe8c0bbecc8158e21ff...

Powered by Google App Engine
This is Rietveld 408576698