New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 302 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2011
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug
Team-Accessibility

Blocked on:
issue 37715
issue 47141

Blocking:
issue 65537

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
link

Issue 17951: cmd-ctrl-d should show dictionary entry of currently highlighted word

Reported by thakis@chromium.org, Jul 29 2009 Project Member

Issue description

Chrome Version       : 3.0.195.1
URLs (if applicable) :
OS version               : 10.5.7

What steps will reproduce the problem?
1. Highlight a word
2. Hit cmd-ctrl-d

What is the expected result?

The dictionary definition of the currently highlighted word should show up, 
just as in every other app (e.g. safari, colloquy, ...).

What happens instead?

Nothing.


I'm not sure if this will be fixed when cmd-a and friends work, hence a 
separate bug.
 

Comment 1 Deleted

Comment 2 Deleted

Comment 3 by glow...@gmail.com, Sep 28 2009

Just for the record, the word doesn't need to be highlight for the dictionary to pop up. 
You only need to hover the cursor over the word. I'm not sure if this will change the 
implementation at all, but erring on the side of caution.

Comment 4 Deleted

Comment 5 by scherkus@chromium.org, Oct 29 2009

I want this BADLY :(

Could someone give me a 10-second overview and I can try to find time to implement this?

Comment 6 by thakis@chromium.org, Oct 30 2009

scherkus: Cool :-) I did some digging, just for you.

If you hit cmd-ctrl-d in a current ToT build, you get a NOTIMPLEMENTED in render_widget_host_view_mac.mm, method 
characterIndexForPoint:. dtrace tells us who calls that:

~/src/chrome-git/src thakis$ sudo dtrace -n 'objc$target::-characterIndexForPoint?:entry { ustack(); }' -p 16519
dtrace: description 'objc$target::-characterIndexForPoint?:entry ' matched 4 probes
CPU     ID                    FUNCTION:NAME
  1  25654   -characterIndexForPoint::entry 
              AppKit`-[NSInputContext characterIndexForPoint:]
              AppKit`_NSTSMEventHandler+0x1703
              HIToolbox`DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*)+0x49d
              HIToolbox`SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*)+0x195
              HIToolbox`SendEventToEventTarget+0x34
              HIToolbox`SendTSMEvent+0xf7
              HIToolbox`SendTextInputEvent+0xad1
              DictionaryService`DSGetTextUnderMouse+0x11a
              DictionaryService`DSInitializeMessageReceiving+0x2d0
              CoreFoundation`__CFMessagePortPerform+0xc1
              CoreFoundation`CFRunLoopRunSpecific+0xf38
              CoreFoundation`CFRunLoopRunInMode+0x58
              HIToolbox`RunCurrentEventLoopInMode+0x11b
              HIToolbox`ReceiveNextEventCommon+0x176
              HIToolbox`BlockUntilNextEventMatchingListInMode+0x6a
              AppKit`_DPSNextEvent+0x291
              AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]+0x80
              AppKit`-[NSApplication run]+0x31b
              Google Chrome Framework`ChromeMain+0x495013
              Google Chrome Framework`ChromeMain+0x4950f6

Googling for DSGetTextUnderMouse() suggests that DictionaryService tries to get the actual text under the cursor using the 
accessibility system APIs ("DSAXGetTextUnderMouse"). So it looks like dictionary's logic goes like:

1.) Get index of character below cursor using characterIndexForPoint:
2.) Use the accessibility APIs to get the text at that index
3.) Display the window you want so badly

To make this work, you have to plumb the coordinate->index transformation through IPC to make 1 work and then probably 
implement enough of the accessibility stuff to make 2. work (this would be a good thing either way).

cc'ing dmac cause he seems to have some experience with AX stuff. Maybe he can comment on your next steps once you're done with 
1. :-)

Comment 7 by thakis@chromium.org, Nov 3 2009

Fixing this might help with  issue 20868  .

Comment 8 Deleted

Comment 9 by rsesek@chromium.org, Nov 5 2009

Also be aware of  issue 19803 , which is adding this functionality to the context menu.

Comment 10 by jankass...@gmail.com, Dec 3 2009

Note that it's not necessarily one word. Even without selection "Las Vegas" for instance 
is looked up together if you hit CTRL+CMD+d while hovering the two words without 
having them selected.

Comment 11 Deleted

Comment 12 Deleted

Comment 13 Deleted

Comment 14 by danno@google.com, Feb 4 2010

I see there hasn't been activity here for a while, so I would like to tackle this issue myself starting with 1), 
unless scherkus has made significant progress on this already. 

Forgive my newbie ignorance, but I assume characterIndexForPoint needs the index as an "absolute" index for 
the Cocoa view/WebWidget: it seems like the trick is converting the text index from the DOM node (I see from 
the Double-Click handling code that selects a word how to get a Position/Selection from a location in the 
window) into a value that is unique among all nodes in the widget. My naive approach would be to walk the 
DOM node chain backward from the mouse-hovering-over node, adding on the text lengths of all of the 
text-containing nodes to the "relative" index obtained from the node where the mouse is hovering. The end 
result would uniquely identify the index of the character even between nodes. Other ideas? Objections?

Comment 15 by jeremy@chromium.org, Feb 4 2010

danno: (sorry if this comment is redundant) I'd check how Safari implements this and try 
to use the same code path if possible.

Comment 16 Deleted

Comment 17 by bau...@google.com, Feb 4 2010

What's also interesting is that this works out of the box in any Cocoa application
using a WebView (which Chrome probably doesn't use directly). So I think almost all
of this functionality is already implemented somewhere in WebKit.

Comment 18 by danno@google.com, Feb 4 2010

MiniBrowser in Apple's DevTools examples does the right thing: it pops up the dictionary during the hover, and 
even does the right thing for "Las Vegas". A little poking around in gdb reveals that accessibilityHitTest gets 
called in WebHTMLView but never characterIndexForPoint. So it indeed seems that the mac Cocoa WebKit classes 
implement this simply by providing the right pieces of the NSAccessibility API.

Comment 19 by viettrungluu@chromium.org, Feb 4 2010

The nature of the problem for us is that, since our renderer lives in another process, we must do all of 
this over IPC. Moreover, we must also do this asynchronously. Accessibility for web content is similarly 
hard for us.

Comment 20 Deleted

Comment 21 by xlu.2...@gmail.com, Feb 5 2010

I want to add that Chrome for Windows also has this problem. 

I've tried these dictionary programs: lingoes, babylon, Kingsoft Powerword, and 
Youdao dictionary. All the dictionaries work well with IE and Firefox: if the mouse 
is hovering over a word, the definition of the word will pop up.None can work with 
Chrome.

I suspect it has something to do with the way Chrome renders text. Why doesn't Chrome 
use the normal Windows way to render text?

Comment 22 by thakis@chromium.org, Feb 6 2010

Labels: -Area-Misc Area-Feature
As trungl says, implementing this is probably nearly the same as implementing a11y for 
web content. http://dev.chromium.org/developers/design-documents/accessibility 
describes how chrome/windows does this (section "MSAA for web content", apparently 
they use sync ipcs?? Maybe we can do better on OS X?).

A11y for web contents for chrome/mac is  issue 27112 , which is owned by feldstein. 
Maybe talk to him if he's done any work there yet. A good first step is probably to write 
up a high-level design doc for web a11y.

Comment 23 Deleted

Comment 24 by dhw@chromium.org, Feb 10 2010

 Issue 31603  has been merged into this issue.

Comment 25 Deleted

Comment 26 by viettrungluu@chromium.org, Feb 20 2010

+feldstein since he's working on related accessibility stuff. It's hard though.

Comment 27 by karen@chromium.org, Mar 9 2010

Labels: wasm5kg Mstone-X

Comment 28 Deleted

Comment 29 by Deleted ...@, May 18 2010

This may help somewhat: [attached file] lines 473 to 511 re http://github.com/sdegutis/The-Gist
StatusMessageTextField.m
16.2 KB View Download

Comment 30 by dhw@chromium.org, May 20 2010

Blockedon: 27112

Comment 31 by redgieme...@gmail.com, Aug 17 2010

Any progress on this. This is the main reason I don't use Chrome as my main browser despite having features I love about it.

Comment 32 by yoshtal...@gmail.com, Aug 17 2010

@redgiemental: you can install the dictionary add-on/extention, which gives you the ability to define words by pressing cmd+double-clicking on the word. It's not cmd-ctrl-D, but it's a happy medium and it got me using Chrome.

Comment 33 by pinkerton@chromium.org, Aug 17 2010

We discussed this at our meeting yesterday. I think the status is that we have access to the text a11y info from the renderer now, but we still don't have the mapping from mouse location to word text hooked up. I believe that's the last big gap that needs bridging for all this to work.

Someone can correct me if I'm wrong.

Comment 34 by redgieme...@gmail.com, Aug 17 2010

Will it have the contextual menu option as well? I generally use that rather than a key combo. 

I actually prefer the definition popping in Mac OS' separate dictionary app that the pop over you get with the add on. I know in Safari you get the pop over with the short cut and the separate app opens with the contextual menu. 

I understand if you don't want Chrome to function exactly like Safari in this regard and it is your decision to make.I would have posted in the thread about the contextual menu option but it's listed as a duplicate of this thread.

Comment 35 by mikesmith@chromium.org, Aug 18 2010

 Issue 46153  has been merged into this issue.

Comment 36 by annapop@chromium.org, Aug 21 2010

Labels: not-sync

Comment 37 by emrecano...@gmail.com, Aug 31 2010

This is pretty much the only thing Chrome lacks on Mac now.

Comment 38 Deleted

Comment 39 by garrettl...@gmail.com, Sep 2 2010

please please pleasee add this. i love chrome but i can't browse without services!

Comment 40 by rsesek@chromium.org, Sep 2 2010

Labels: Restrict-AddIssueComment-Commit

Comment 41 by thakis@chromium.org, Sep 2 2010

garettlee09: Support for services is being added to chrome 7, the next stable release. It's already in the dev channel. This (cmd-ctrl-d) requires a bit more than services, see comment 33.

Comment 42 by ctguil@chromium.org, Sep 12 2010

Labels: Feature-Accessibility

Comment 43 by rsesek@chromium.org, Sep 27 2010

 Issue 56923  has been merged into this issue.

Comment 44 by stuartmorgan@chromium.org, Oct 4 2010

 Issue 57621  has been merged into this issue.

Comment 45 by thakis@chromium.org, Dec 6 2010

rsesek: I think characterIndexForPoint isn't called because attributedSubstringFromRange: returns nil. The attached program has dummy implementations for NSTextInput and NSTextInputClient that log which functions are called and that returns dummy values (hit cmd-ctrl-d over the left and right half of the window). The output for NSTextInput:

2010-12-05 21:11:42.504 dictionary[21862:a0f] <textinput: 0x10051b030> attributedSubstringFromRange:: {0, 10}
2010-12-05 21:11:42.504 dictionary[21862:a0f] <textinput: 0x10051b030> selectedRange
2010-12-05 21:11:42.505 dictionary[21862:a0f] <textinput: 0x10051b030> characterIndexForPoint:: {401, 361}
2010-12-05 21:11:42.505 dictionary[21862:a0f] <textinput: 0x10051b030> attributedSubstringFromRange:: {0, 50}
2010-12-05 21:11:42.507 dictionary[21862:a0f] <textinput: 0x10051b030> firstRectForCharacterRange:: {0, 6}

If attributedSubstringFromRange: returns nil, nothing happens. With a dummy result, the definition for the dummy result is shown.
dictionary.zip
22.7 KB Download

Comment 46 by rsesek@chromium.org, Dec 6 2010

Blockedon: 37715

Comment 47 by rsesek@chromium.org, Dec 16 2010

Status: Started
I'm about 2/rds of the way done on this. Hope is to make M10.

Comment 48 by suzhe@chromium.org, Dec 16 2010

How would you solve this issue? I think it's related to issue 55130.

Comment 49 by rsesek@chromium.org, Dec 16 2010

See this page for a general overview of how it's going to work:
http://www.chromium.org/developers/design-documents/system-dictionary-pop-up-architecture

And while you're free to insert your own denominator for comment 47, the correct answer is 3.

Comment 50 by suzhe@chromium.org, Dec 16 2010

I think this trick also works for issue 55130. I'm just wondering if it's really ok for blocking the browser process, even for only 1.5 seconds.

Comment 51 by rsesek@chromium.org, Dec 16 2010

It's the only way to do this, unfortunately. The design doc is a bit out of date, but I'll update it when it's finished. 1.5 seconds is a gross overestimate; my experiments locally indicate that the roundtrip can happen under 1 millisecond most of the time. I've added UMA histograms so that when this ships we can get the best upper-bound for these timeouts using real world data.

Comment 52 by suzhe@chromium.org, Dec 16 2010

Great. Looking forward to your CL.

Comment 53 by stuartmorgan@chromium.org, Jan 14 2011

 Issue 69620  has been merged into this issue.

Comment 54 by rsesek@chromium.org, Jan 20 2011

Blockedon: -27112
Labels: -Mstone-X Mstone-11
Good news: CL is out for review.

Bad news: Review isn't going to be done by Monday, which is when the M10 train leaves the station.

Good news: It will be in M11!

Comment 55 by rsesek@chromium.org, Jan 22 2011

Blockedon: 47141

Comment 56 by rsesek@chromium.org, Jan 27 2011

Labels: -HelpWanted -wasm5kg

Comment 57 by kareng@google.com, Mar 9 2011

Labels: -Mstone-11 MovedFrom-11 Mstone-12
rolling non releaseblocker mstone 11 bugs to mstone 12.

Comment 58 by rsesek@chromium.org, Apr 8 2011

The first of two WebKit patches landed upstream: http://trac.webkit.org/changeset/83081. Second patch should hopefully be landing next week, with the Chromium part to follow as soon as the WebKit roll picks it up.

Comment 59 by rsesek@chromium.org, Apr 15 2011

Labels: -Mstone-12 Mstone-13

Comment 60 by bugdroid1@chromium.org, May 2 2011

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=83723

------------------------------------------------------------------------
r83723 | rsesek@chromium.org | Mon May 02 08:46:01 PDT 2011

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_view.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_mac.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_widget.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_mac.mm?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_views.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/view_messages.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/ipc/ipc_message_utils.h?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/text_input_client_mac_unittest.mm?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/text_input_client_message_filter.h?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/text_input_client_mac.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/common_param_traits.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/font_descriptor_mac_unittest.mm?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/font_descriptor_mac.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_renderer.gypi?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_gtk.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_message_filter.cc?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/attributed_string_coder_mac_unittest.mm?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/attributed_string_coder_mac.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_gtk.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_view_host.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_tests.gypi?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/content_common.gypi?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/text_input_client_message_filter.mm?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/render_widget_host_view_views.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/chrome_content_renderer_client.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_content_browser_client.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_common.gypi?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/text_input_client_observer.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_view_host.h?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/text_input_client_messages.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host.h?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/attributed_string_coder_mac.mm?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/text_input_client_messages.h?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/font_descriptor_mac.mm?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/render_widget_fullscreen_pepper.cc?r1=83723&r2=83722&pathrev=83723
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/common_param_traits.cc?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/text_input_client_mac.mm?r1=83723&r2=83722&pathrev=83723
 A http://src.chromium.org/viewvc/chrome/trunk/src/chrome/renderer/text_input_client_observer.cc?r1=83723&r2=83722&pathrev=83723

[Mac] Implement the system dictionary popup by implementing NSTextInput methods.

This is a two-sided patch; the Chromium side is plumbing to marshall data from
the renderer to the system APIs.

Note that just hitting Cmd+Ctrl+D usually does not bring up the popup. I think
this may be an Apple bug, but I have not yet found a work-around.

BUG= 17951 , 37715 , 47141 
TEST=Hold Cmd+Ctrl+D on a web page and mouse around. The dictionary popup should follow the mouse and show the definition of the current word.
TEST=In a text area, the dictionary popup should work only if the text area has focus.
R=avi,suzhe,jam

Review URL: http://codereview.chromium.org/6289009
------------------------------------------------------------------------

Comment 61 by rsesek@chromium.org, May 3 2011

Status: Fixed
This is now done for Chrome 13 :-). There's a few minor issues remaining, but the bulk of the work is done and you can now lookup words using Cmd+Ctrl+D.

Comment 62 by asvitk...@chromium.org, Aug 4 2012

Blocking: chromium:65537

Comment 63 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Blockedon: -chromium:37715 -chromium:47141 chromium:37715 chromium:47141
Blocking: -chromium:65537
Labels: -Area-UI -Feature-Accessibility -Mstone-13 Cr-UI-Accessibility Cr-UI M-13

Comment 64 by bugdroid1@chromium.org, Mar 13 2013

Project Member
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue

Sign in to add a comment