Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 139108 Candidate windows is shown in wrong place in Retina display
Starred by 1 user Project Member Reported by horo@chromium.org, Jul 26, 2012 Back to list
Status: Fixed
Owner: horo@chromium.org
Closed: Jul 2012
Cc: nona@chromium.org, kenjibaheux@chromium.org, tha...@chromium.org
Components:
OS: Mac
Pri: 2
Type: Bug

Restricted
  • Only users with EditIssue permission may comment.


Sign in to add a comment
Version: 22.0.1217.0 canary
OS: MacOS X 10.7.4 and 10.8

What steps will reproduce the problem?
1. open www.google.com
2. enable Kotoeri
3. enter "a" + space

What is the expected output? What do you see instead?
Candidate windows is shown in wrong place in Retina display

Please use labels and text to provide additional information.

 
Comment 1 by horo@chromium.org, Jul 26, 2012
wrong_candidate_window_pos.png
2.3 MB View Download
Labels: hotlist-Japan
Thanks!
Comment 3 by nona@chromium.org, Jul 26, 2012
Cc: tha...@chromium.org
Status: Started
+Nico

Unfortunately I don't have retina mac pro, but I may find the matter of cause.
According to the documentation, we should not use deprecated APIs for coordinates calculation. 

I will change to use convertRectToBacking/convertRectToScreen instead of convertRectToBase/convertScreenToBase if possible.

http://developer.apple.com/library/mac/documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/HighResolutionOSX.pdf

Comment 4 by tha...@chromium.org, Jul 26, 2012
Labels: Feature-HighDPI macdpi
That's likely wrong. You want convertRect:rect toView:nil instead if convertRectToBase: is used anywhere at the moment. See https://groups.google.com/a/chromium.org/forum/?fromgroups#!topic/chromium-dev/7kN3KagcwNg for more information.
Comment 5 by tha...@chromium.org, Jul 26, 2012
You don't need a retina mac to test this by the way, you only need to run OS X 10.7 (which was released long ago). See http://developer.apple.com/library/mac/#documentation/GraphicsAnimation/Conceptual/HighResolutionOSX/Testing/Testing.html#//apple_ref/doc/uid/TP40012302-CH6-SW2 for how to test retina mode on a "normal" mac.
Comment 6 by nona@chromium.org, Jul 26, 2012
Oh, thank you for good information!

I will try it!
Comment 7 by nona@chromium.org, Jul 27, 2012
Cc: nona@chromium.org
reassigning to horo@

He kindly take on this issue.
Comment 8 by nona@chromium.org, Jul 27, 2012
Owner: horo@chromium.org
Status: Assigned
Comment 9 by a...@chromium.org, Jul 27, 2012
If the code change involves fixing coordinate conversion, please assign me for review.
Comment 10 by horo@chromium.org, Jul 29, 2012
This cl https://chromiumcodereview.appspot.com/10825070 will fix the problem.

cn_after.png
1.4 MB View Download
ja_after.png
1.7 MB View Download
ja_before.png
2.2 MB View Download
cn_before.png
1.7 MB View Download
Comment 11 by tha...@chromium.org, Jul 29, 2012
Status: Started
Your CL also fixes the bubble that appears if you keep eg "a" pressed with a US keyboard layout in lion or mountain lion. Thanks!
Comment 12 by tha...@chromium.org, Jul 29, 2012
Labels: Mstone-21 Merge-Requested
Karen, can we take merge this to 1180 for the m21 stable refresh? The fix is a very safe one line "well, duh" CL. (I realize the CL hasn't even landed yet, but since it just changes a coordinate computation, it's virtually impossible for it to have a stability impact).
Comment 13 by kareng@google.com, Jul 29, 2012
let's go to stable first and then we can consider it :)
Project Member Comment 14 by bugdroid1@chromium.org, Jul 29, 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=148916

------------------------------------------------------------------------
r148916 | horo@chromium.org | 2012-07-29T18:09:47.791867Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.mm?r1=148916&r2=148915&pathrev=148916

Fix the candidate window position of IME in Retina display.

BUG= 139108 
TEST=manually done


Review URL: https://chromiumcodereview.appspot.com/10825070
------------------------------------------------------------------------
Comment 15 by tha...@chromium.org, Jul 30, 2012
Fix confirmed on canary, in both lodpi and hidpi.
Comment 16 by kareng@google.com, Jul 30, 2012
Labels: -Merge-Requested Merge-Approved
WOOHOO!

Thanks horo@, nona@ and everyone!
Comment 18 Deleted
The issue is not reproducible in Mac 10.7.4 & Mac 10.8 ( Retina ) .
Build Used : 22.0.1221.0


Comment 20 by tha...@chromium.org, Jul 31, 2012
Status: Fixed
I'll merge this to m21 once the first stable push has happened.
Project Member Comment 21 by bugdroid1@chromium.org, Aug 1, 2012
Labels: -Merge-Approved merge-merged-1180
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=149354

------------------------------------------------------------------------
r149354 | thakis@chromium.org | 2012-08-01T02:15:11.652222Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1180/src/content/browser/renderer_host/render_widget_host_view_mac.mm?r1=149354&r2=149353&pathrev=149354

Merge 148916 - Fix the candidate window position of IME in Retina display.

BUG= 139108 
TEST=manually done


Review URL: https://chromiumcodereview.appspot.com/10825070

TBR=horo@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10827110
------------------------------------------------------------------------
Project Member Comment 22 by bugdroid1@chromium.org, Oct 13, 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 23 by bugdroid1@chromium.org, Mar 10, 2013
Labels: -Area-UI -Feature-TextInput -Feature-HighDPI -Mstone-21 Cr-UI Cr-UI-HighDPI M-21 Cr-UI-Input-Text-IME
Project Member Comment 24 by bugdroid1@chromium.org, Mar 14, 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Sign in to add a comment