Last Comment Bug 301451 - [meta] Gecko doesn't support Cmd-Ctrl-D lookup in Mac OS X Dictionary.app
: [meta] Gecko doesn't support Cmd-Ctrl-D lookup in Mac OS X Dictionary.app
Status: NEW
[parity-Safari][parity-Chrome][lang=o...
: meta
Product: Core
Classification: Components
Component: Widget: Cocoa (show other bugs)
: Trunk
: All Mac OS X
: -- enhancement with 75 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors: Stephen A Pohl [:spohl]
: 310874 323556 328153 363340 376098 383254 397317 435618 566161 626566 787092 837451 952958 (view as bug list)
Depends on: 1116391 1145224 308471 352329 569334 1123625 1177943
Blocks: maca11y 687026 778544
  Show dependency treegraph
 
Reported: 2005-07-20 08:19 PDT by Prachi Gauriar
Modified: 2016-06-08 11:23 PDT (History)
73 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
wip (4.33 KB, patch)
2010-05-16 04:14 PDT, Markus Stange [:mstange]
no flags Details | Diff | Splinter Review
v1 (1.15 KB, patch)
2010-06-01 04:58 PDT, Markus Stange [:mstange]
no flags Details | Diff | Splinter Review
v2 (1.14 KB, patch)
2010-07-07 17:33 PDT, Markus Stange [:mstange]
masayuki: review+
Details | Diff | Splinter Review

Description Prachi Gauriar 2005-07-20 08:19:39 PDT
Camino (2005071808) doesn't integrate with Tiger's Dictionary.  In many other
applications (including Safari), if one presses cmd-ctrl-d (US keyboard) while
hovering the mouse over a word, a panel pops up with the definition of that
word.  This doesn't happen in Camino.

The dictionary lookup and panel display is apparently handled by the
PopupDictDaemon, which can be found at
/Applications/Dictionary.app/Contents/SharedSupport/PopupDictDaemon.app.  While
it is unclear how this daemon works, I suspect it uses the Accessibility APIs to
determine what word is under the cursor.  As such, I believe that making Camino
Accessibility friendly would be necessary (and perhaps sufficient) to fix this.

These two links should be helpful.  Baiscally, the browser view just has to
implement the NSAccessibility protocol.

http://developer.apple.com/documentation/Cocoa/Conceptual/Accessibility/index.html
http://developer.apple.com/documentation/Cocoa/Reference/ApplicationKit/ObjC_classic/Protocols/NSAccessibility.html
Comment 1 Prachi Gauriar 2005-07-20 08:21:09 PDT Comment hidden (obsolete)
Comment 2 Jasper 2005-07-20 08:27:32 PDT Comment hidden (obsolete)
Comment 3 Samuel Sidler (old account; do not CC) 2005-07-20 09:00:28 PDT Comment hidden (obsolete)
Comment 4 Prachi Gauriar 2005-07-20 09:13:23 PDT Comment hidden (obsolete)
Comment 5 Samuel Sidler (old account; do not CC) 2005-07-20 09:34:51 PDT Comment hidden (obsolete)
Comment 6 Mike Pinkerton (not reading bugmail) 2005-07-20 09:41:10 PDT Comment hidden (obsolete)
Comment 7 Simon Fraser 2005-07-20 10:43:31 PDT Comment hidden (obsolete)
Comment 8 Smokey Ardisson (offline for a while; not following bugs - do not email) 2005-10-02 22:20:02 PDT
*** Bug 310874 has been marked as a duplicate of this bug. ***
Comment 9 Joachim Noreiko 2005-11-26 08:43:01 PST Comment hidden (obsolete)
Comment 10 Samuel Sidler (old account; do not CC) 2005-12-23 10:42:44 PST Comment hidden (obsolete)
Comment 11 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-01-15 16:03:02 PST
*** Bug 323556 has been marked as a duplicate of this bug. ***
Comment 12 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-01-15 21:15:11 PST
*** Bug 323556 has been marked as a duplicate of this bug. ***
Comment 13 Håkan Waara 2006-03-15 14:47:12 PST Comment hidden (obsolete)
Comment 14 stephen@ju-ju.com 2006-04-20 07:40:29 PDT Comment hidden (obsolete)
Comment 15 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-04-20 10:07:34 PDT Comment hidden (obsolete)
Comment 16 Billy Halsey 2006-07-06 18:24:55 PDT Comment hidden (obsolete)
Comment 17 Simon Fraser 2006-07-06 19:32:30 PDT
No, this is to support the Dictionary popup window that you get when holding Command-Control-D over any word.
Comment 18 Håkan Waara 2006-11-28 06:54:10 PST Comment hidden (obsolete)
Comment 19 Håkan Waara 2006-11-28 06:55:28 PST
*** Bug 328153 has been marked as a duplicate of this bug. ***
Comment 20 Smokey Ardisson (offline for a while; not following bugs - do not email) 2006-12-10 00:29:30 PST
*** Bug 363340 has been marked as a duplicate of this bug. ***
Comment 21 froodian (Ian Leue) 2007-03-31 08:36:29 PDT
*** Bug 376098 has been marked as a duplicate of this bug. ***
Comment 22 Stuart Morgan 2007-06-04 21:56:59 PDT
*** Bug 383254 has been marked as a duplicate of this bug. ***
Comment 23 Stuart Morgan 2007-09-23 22:50:40 PDT
*** Bug 397317 has been marked as a duplicate of this bug. ***
Comment 24 José Jeria 2008-06-24 11:17:03 PDT
*** Bug 435618 has been marked as a duplicate of this bug. ***
Comment 25 Patrick 2008-10-22 14:38:26 PDT
Re comment 18: Håkan, you mention that we'd be a lot closer in fixing this bug when bug 352329 is fixed. That bug was eventually fixed in 2006. 

What is it that remains before this bug can be fixed?
Comment 26 Håkan Waara 2008-10-24 01:35:19 PDT
Last I looked, it was not 100% documented which methods are required for Dictionary.app to work, but it seems that it has to do with different text queries that are sent to the top-most view that it uses in order to find out the word at a specific point. We implement some of these in ChildView (nsChildView.mm), to what extent I'm not sure.

Part of this bug is finding out which methods (probably on ChildView) are required for this... It's too bad Dictionary.app is not open source!
Comment 27 Håkan Waara 2008-10-24 01:38:46 PDT
One more thing:  basically the creator of this bug and others (including me) previously assumed that Dictionary support was tied to Accessibility support, but it turns out that maybe a11y is not a requirement for this to work. 

Maybe Dictionary only uses some of these text callbacks on NSViews to find out the currently selected text. If that's the case, this bug really doesn't depend on bug 157209 (full a11y support on OS X) but can be fixed independently.
Comment 28 Thomas K. (:tom) 2008-10-24 01:51:43 PDT Comment hidden (obsolete)
Comment 29 stephen@ju-ju.com 2008-10-24 05:28:19 PDT Comment hidden (obsolete)
Comment 30 Stephen Wade 2009-02-15 07:47:08 PST Comment hidden (obsolete)
Comment 31 Samuel Sidler (old account; do not CC) 2009-02-17 00:39:16 PST
This bug includes all Mozilla-based applications.
Comment 32 Joe Drew (not getting mail) 2009-04-01 14:47:47 PDT
Does finishing the OS X Services implementation have any bearing on this?
Comment 33 Smokey Ardisson (offline for a while; not following bugs - do not email) 2009-04-01 22:38:56 PDT
(In reply to comment #32)
> Does finishing the OS X Services implementation have any bearing on this?

If Håkan's "new" theory in the second half of comment 27 is correct and if the Services implementation provides them, perhaps.  Otherwise, no ;)  

I looked at ADC not too long ago and still couldn't find any documentation on what the hover-based Cmd-Ctrl-D lookup required to work; it's a black box.  (For instance, OOo Aqua does have Accessibility support that more-or-less works with VoiceOver, Cmd-Ctrl-D does not work there.)
Comment 34 Simon Fraser 2009-04-01 22:56:24 PDT
You have to implement the NSTextInput protocol on one of your NSViews.
Comment 35 Smokey Ardisson (offline for a while; not following bugs - do not email) 2009-09-13 15:21:11 PDT
Sounds like something for ChildView to do, then; thanks, smfr!  --> Widget:Cocoa
Comment 36 Markus Stange [:mstange] 2009-09-22 23:57:30 PDT
Looks like our NSTextInput implementation is already relatively complete - maybe it's easy to add the rest that's required for Dictionary support. Masayuki, do you want to look into this?
Comment 37 Paul Borokhov (lensovet) 2010-01-24 18:09:01 PST Comment hidden (obsolete)
Comment 38 Paul Borokhov (lensovet) 2010-01-24 18:10:37 PST
also, it looks like NSTextInput is deprecated in 10.5 and above, and the NSTextInputClient protocol is its replacement.
Comment 39 Markus Stange [:mstange] 2010-01-25 11:23:27 PST
(In reply to comment #37)
> what happened to this bug?

Nothing. Feel free to pick it up; our NSTextInput implementation is somewhere in nsChildView.mm.

(In reply to comment #38)
> also, it looks like NSTextInput is deprecated in 10.5 and above, and the
> NSTextInputClient protocol is its replacement.

NSTextInputClient was introduced in 10.5, so we can start using it when we've finally decided that we'll truly drop 10.4 support. However, converting to the new interface will be very simple because they're very similar, so there's no need to wait for that before fixing this bug.
Comment 40 Stephen Wade 2010-01-25 17:08:31 PST Comment hidden (advocacy)
Comment 41 Zac Zheng 2010-03-20 03:53:02 PDT
There's a plugin which adds a 'Look Up in Dictionary' context menu for the selected word and opens up Mac OS X's Dictionary.  https://addons.mozilla.org/en-US/firefox/addon/7261
Comment 42 Paul Borokhov (lensovet) 2010-03-20 12:21:25 PDT Comment hidden (off-topic)
Comment 43 Markus Stange [:mstange] 2010-03-20 13:01:27 PDT
You can use MXR to browse Firefox source code and search for file names:
http://mxr.mozilla.org/mozilla-central/find?text=&kind=text&string=nschildview.mm

I'd start with adding a "#define DEBUG_IME 1" which will printf messages whenever an NSTextInput method is called. Then I'd check which methods are called when pressing Cmd+Ctrl+D and add implementations to those that aren't implemented yet.
Comment 44 Paul Borokhov (lensovet) 2010-03-20 13:11:44 PDT Comment hidden (off-topic)
Comment 45 Markus Stange [:mstange] 2010-03-20 13:16:46 PDT Comment hidden (off-topic)
Comment 46 Nomis101 2010-05-01 11:55:08 PDT Comment hidden (off-topic)
Comment 47 Aaron Train [:aaronmt] 2010-05-13 10:39:42 PDT
I'm wondering if the recent observance and tackling of the '100 Papercuts' project would consider the inclusion of this bug, see [1] 


[1] https://wiki.mozilla.org/Firefox/Projects/UX_Priorities_3.7
[2] http://limi.net/articles/firefox-ux-team-update-8/
Comment 48 philippe (part-time) 2010-05-15 21:42:08 PDT
*** Bug 566161 has been marked as a duplicate of this bug. ***
Comment 49 Markus Stange [:mstange] 2010-05-16 04:14:02 PDT
Created attachment 445588 [details] [diff] [review]
wip

This is a rough start that only works in focused text fields.
Comment 50 Masayuki Nakano [:masayuki] (Mozilla Japan) 2010-05-16 04:38:37 PDT
> -  nsIWidget* rootWidget = mGeckoChild->GetTopLevelWidget();
> -  NSWindow* rootWindow =
> -    static_cast<NSWindow*>(rootWidget->GetNativeData(NS_NATIVE_WINDOW));
> -  NSView* rootView =
> -    static_cast<NSView*>(rootWidget->GetNativeData(NS_NATIVE_WIDGET));
> -  if (!rootWindow || !rootView)
> -    return rect;
> +  NSRect rect = NSZeroRect;
>    GeckoRectToNSRect(r, rect);
> -  rect = [rootView convertRect:rect toView:nil];
> -  rect.origin = [rootWindow convertBaseToScreen:rect.origin];
> +  rect = [self convertRect:rect toView:nil];
> +  rect.origin = [[self window] convertBaseToScreen:rect.origin];

Why is this change needed? Doesn't this change break the method behavior on XUL panel?

> characterIndexForPoint

You can use NS_QUERY_CHARACTER_AT_POINT event. See the patch of bug 308471. I cannot land the patch soon for IME because there are some issues. But you can use a part of my patch for this bug.
Comment 51 Markus Stange [:mstange] 2010-05-16 05:07:55 PDT
(In reply to comment #50)
> > -  nsIWidget* rootWidget = mGeckoChild->GetTopLevelWidget();
> > -  NSWindow* rootWindow =
> > -    static_cast<NSWindow*>(rootWidget->GetNativeData(NS_NATIVE_WINDOW));
> > -  NSView* rootView =
> > -    static_cast<NSView*>(rootWidget->GetNativeData(NS_NATIVE_WIDGET));
> > -  if (!rootWindow || !rootView)
> > -    return rect;
> > +  NSRect rect = NSZeroRect;
> >    GeckoRectToNSRect(r, rect);
> > -  rect = [rootView convertRect:rect toView:nil];
> > -  rect.origin = [rootWindow convertBaseToScreen:rect.origin];
> > +  rect = [self convertRect:rect toView:nil];
> > +  rect.origin = [[self window] convertBaseToScreen:rect.origin];
> 
> Why is this change needed? Doesn't this change break the method behavior on XUL
> panel?

Ah, panels... I see. It's not needed, I just wanted to simplify the code.

> > characterIndexForPoint
> 
> You can use NS_QUERY_CHARACTER_AT_POINT event. See the patch of bug 308471. I
> cannot land the patch soon for IME because there are some issues. But you can
> use a part of my patch for this bug.

Oh, good!
Comment 52 Markus Stange [:mstange] 2010-06-01 04:58:26 PDT
Created attachment 448502 [details] [diff] [review]
v1

With the patch for bug 308471, all we need to do here is implement the textStorage method (which isn't even part of the NSTextInput protocol, but of NSTextView).
This will at the moment only work for focused text boxes. Moreover, the text that the dictionary popup overlays on top of the targeted word will have the wrong font and font size. That's because the NSAttributedStrings we return only contain plain text.
Comment 53 Markus Stange [:mstange] 2010-06-01 05:40:11 PDT
I've filed bug 569331 and bug 569334 on the focus and visual issues.
Comment 55 Markus Stange [:mstange] 2010-06-01 08:26:46 PDT
Indeed!
Comment 56 Markus Stange [:mstange] 2010-07-07 17:33:11 PDT
Created attachment 456363 [details] [diff] [review]
v2

with NSTextStorage instead of NSAttributedString
Comment 57 José Jeria 2010-12-03 15:12:36 PST
Will this land in time for Firefox 4? This was reviews back in July
Comment 58 Markus Stange [:mstange] 2010-12-08 09:37:27 PST
I haven't landed this yet because I haven't had a chance to look into bug 569331. The current implementation only works for focused elements, and I think landing it in this state would make for a quite frustrating experience.
Comment 59 cr4shing 2011-03-10 21:39:08 PST Comment hidden (off-topic)
Comment 60 Ricky Kung 2011-07-01 07:52:48 PDT Comment hidden (advocacy)
Comment 61 Chris Lawson (gone) 2011-07-01 08:16:17 PDT Comment hidden (advocacy)
Comment 62 Jo Hermans 2011-09-16 08:09:59 PDT
*** Bug 626566 has been marked as a duplicate of this bug. ***
Comment 63 Xander S 2012-03-28 17:55:00 PDT Comment hidden (advocacy)
Comment 64 Chris Lawson (gone) 2012-03-28 19:05:46 PDT Comment hidden (advocacy)
Comment 65 philippe (part-time) 2013-02-02 22:17:41 PST
*** Bug 837451 has been marked as a duplicate of this bug. ***
Comment 66 todor.dragnev 2013-03-17 22:38:51 PDT Comment hidden (advocacy)
Comment 67 Jared Wein [:jaws] (please needinfo? me) 2013-04-19 13:14:45 PDT
I have found the documentation for this on Apple's website: https://developer.apple.com/library/mac/#releasenotes/Cocoa/AppKit.html#//apple_ref/doc/uid/TP30000741

---------------

 Quick Look Gesture
There is now API to receive the Quick Look gesture (also known as the dictionary look up gesture). The user can perform this gesture with a three finger single tap, or the appropriate keyboard shortcut (Cmd-Ctl-D by default). There are two new NSResponder methods: an event method and an action method.

/* Perform a Quick Look on the content at location in the event. If there are no Quick Look items at the location, call super.
   Also, see quickLookPreviewItems: further below.
*/
- (void)quickLookWithEvent:(NSEvent *)event;

/* Perform a Quick Look on the text cursor position, selection, or whatever is appropriate for your view. If there are no Quick Look
   items, then call [[self nextResponder] tryToPerform:_cmd with:sender]; to pass the request up the responder chain. Eventually
   AppKit will attempt to perform a dictionary look up. Also see quickLookWithEvent: above.
*/
- (void)quickLookPreviewItems:(id)sender;

To support the new event responder method, there is also a new NSEventType, NSEventTypeQuickLook. The only valid properties of and NSEventTypeQuickLook event are: -locationInWindow and -modifiers. A quick look event does not come in through the normal event mechanism, therefore there is no corresponding event mask for it, nor should you attempt to look for it in sendEvent: or with nextEventMatchingMask:… methods.

NOTE: NSApplication's default implementation of quickLookPreviewItems:, will perform a dictionary lookup. NOTE: There is a bug that causes the quickLoop responder methods to jump from NSWindow directly to NSApp bypassing NSWindowController, NSDocument, and NSAppDelegate.

-------------------------------

Stephen, would you be willing to mentor this bug?
Comment 68 Stephen A Pohl [:spohl] 2013-04-19 13:42:46 PDT
(In reply to Jared Wein [:jaws] from comment #67)
> Stephen, would you be willing to mentor this bug?

Sure. :-)
Comment 69 Byron Jones ‹:glob› [PTO until 2016-10-10] 2013-08-21 07:33:17 PDT
*** Bug 787092 has been marked as a duplicate of this bug. ***
Comment 70 nicolas.brouard 2013-09-02 02:02:22 PDT Comment hidden (advocacy)
Comment 71 Marc Bejarano 2014-01-05 14:57:09 PST
*** Bug 952958 has been marked as a duplicate of this bug. ***
Comment 72 frankfang1990 2014-01-18 08:47:35 PST Comment hidden (advocacy)
Comment 73 Marc Bejarano 2014-07-08 13:09:27 PDT Comment hidden (off-topic)
Comment 74 J. Ryan Stinnett [:jryans] (use ni?) 2014-07-08 13:11:43 PDT Comment hidden (off-topic)
Comment 75 Stephen A Pohl [:spohl] 2014-07-28 09:56:28 PDT
I've spent some time last week to read up on the history of this bug and to understand what still needs to be done. Unfortunately, comment 67 turned out to be a bit of a red herring. The Quick Look gesture API is only available on 10.8 and above, which is very limiting for us. More importantly however, its main purpose is to override the default lookup gesture behavior, which is the dictionary lookup. Since we're specifically trying to make the default dictionary lookup work reliably, this API is of no use to us.

To make the dictionary lookup work, we basically need to do three things:
1. Implement NSTextInput -characterIndexForPoint. Tracked in bug 308471.
2. No longer constrain content query events to focused elements. Tracked in bug 569331.
3. Get the text styling right for the dictionary lookup.

Since the first two steps already have dedicated bugs, I'm inclined to use this bug to track the work for the proper text styling. We should also be able to land each of these steps in isolation, which should gradually improve our dictionary lookup experience.
Comment 76 Stephen A Pohl [:spohl] 2014-07-28 09:58:54 PDT Comment hidden (obsolete)
Comment 77 Stephen A Pohl [:spohl] 2014-07-28 12:17:43 PDT
(In reply to Stephen Pohl [:spohl] from comment #75)
> To make the dictionary lookup work, we basically need to do three things:
> 1. Implement NSTextInput -characterIndexForPoint. Tracked in bug 308471.
> 2. No longer constrain content query events to focused elements. Tracked in
> bug 569331.
> 3. Get the text styling right for the dictionary lookup.
> 
> Since the first two steps already have dedicated bugs, I'm inclined to use
> this bug to track the work for the proper text styling.

Turns out that I missed the dependent bug 569334, which is already tracking the work for proper font styling. It looks like this bug here is merely a meta bug at this point.
Comment 78 Xidorn Quan [:xidorn] (UTC+10) 2014-10-21 15:05:48 PDT
This bug works for me in the latest Nightly. Does anyone know what bug do we now track for that implementation?
Comment 79 Masayuki Nakano [:masayuki] (Mozilla Japan) 2014-10-21 19:44:04 PDT
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #78)
> This bug works for me in the latest Nightly. Does anyone know what bug do we
> now track for that implementation?

This is now a meta bug. All depending bugs should be fixed (or removed from the list) before closing this bug.
Comment 80 Nathan Yee [:nyee] 2014-12-29 20:44:52 PST
I can confirm that Cmd-Ctrl-D and the three-finger-tap gesture successfully opens the Dictionary popover with the caveats detailed in Bug 569331 and Bug 569334. Should we create a bug regarding the lack of a "Look up in Dictionary" context menu entry, or at least make that bug blocked until Bug 569331 is fixed?
Comment 81 Stephen A Pohl [:spohl] 2014-12-29 21:06:40 PST
(In reply to Nathan Yee [:nyee] from comment #80)
> Should we create a bug regarding the lack of a "Look up in
> Dictionary" context menu entry, or at least make that bug blocked until Bug
> 569331 is fixed?

Filing a bug for the lack of a context menu entry sounds like a great idea, and making it block this bug here (bug 301451) seems like the right thing to do since this bug is a meta bug, tracking all the work related to dictionary lookup. Thank you!
Comment 82 Nathan Yee [:nyee] 2014-12-29 22:16:21 PST
I have done the bug report. It would be much appreciated if :spohl or someone else can check and confirm it.

Note You need to log in before you can comment on or make changes to this bug.