301451 - [meta] Gecko doesn't support Cmd-Ctrl-D lookup in Mac OS X Dictionary.app

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, enhancement)

Description

[Prachi Gauriar]

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]

Would it be worthwhile to open a bug to make sure all views in Camino are
Accessibility API-friendly?

Comment 2

[Jasper]

Sounds like a good plan?

Comment 3

[Samuel Sidler (old account; do not CC)]

To do this, Camino would have to use Quartz to render pages. This isn't going to
happen for a *long* time. Maybe Cairo will fix this, but I wouldn't count on it.

Comment 4

[Prachi Gauriar]

Samuel: Where did you see that using Quartz for rendering is required?  I found
no mention of it in the docs.

Comment 5

[Samuel Sidler (old account; do not CC)]

(In reply to comment #4)
> Samuel: Where did you see that using Quartz for rendering is required?  I found
> no mention of it in the docs.

Go to the location or search bar, type in a word and then press the short cut
keys (command-ctrl-d). You'll notice that it works fine there (the dictionary
pops up). That's because they are both native widgets and, thus, rendered with
Quartz.

I'm not sure if it's "required" that we use Quartz, but we get the dictionary
automatically by using it and it will take a lot of work to do it otherwise.

Comment 6

[Mike Pinkerton (not reading bugmail)]

no, quartz is a red-herring here. they're native widgets, yes, but they support
whatever API is necessary to do the "right thing". how we draw doesn't affect
it. it's just bits on the screen. we need to be able to tell whoever asks what
we're drawing.

Comment 7

[Simon Fraser [no longer active]]

Supporting accessibility in the content area is a lot of work. We have a whole
XP framework for it (built for MSAA) which might doesn't fit the Mac APIs very well.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** Bug 310874 has been marked as a duplicate of this bug. ***

Comment 9

[Joachim Noreiko]

Could this bug be expanded to include Firefox on OS X, or is a separate bug report required?

Comment 10

[Samuel Sidler (old account; do not CC)]

(In reply to comment #7)
> Supporting accessibility in the content area is a lot of work. We have a whole
> XP framework for it (built for MSAA) which might doesn't fit the Mac APIs very well.

Simon, is this something that needs to be "fixed" to work well with the Mac APIs? And, if so, are there bugs filed regarding this so that we can get it on the Gecko 1.9 branch?

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

*** 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)]

*** Bug 323556 has been marked as a duplicate of this bug. ***

Comment 13

[Håkan Waara]

So, if this dictionary-thingy need the accessibility APIs in order to work, this depends on our core using those APIs.

Marking dependency on the Core bug for this, bug 157209.

Comment 14

[[e-mail]]

I am not so sure it's Accessibility API. Unless BBEdit just turned Cocoa, it supports dictionary popup in it's editor window, which I believe is still Carbon. TSM events are more likely the way. I don't know which events are required but someone posted in Carbon-dev list seems to have pretty good idea:
http://lists.apple.com/archives/carbon-dev/2005/Dec/msg00797.html

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

There are Carbon Accessibility APIs, too; it's not a Carbon-Cocoa issue.

The issue for Gecko is comment 6 and comment 7; text in most of the content area (not text you enter in textboxes/fields, where TSM could apply) is opaque to whatever the Dictionary uses to grab the text.

Comment 16

[Billy Halsey]

Is this a duplicate of bug 151040, which has been fixed in the MOZILLA_1_8_BRANCH to be included in Camino 1.1? If not, there should be at least some part of that bug that could serve as a model for how to implement a fix for this one.

Comment 17

[Simon Fraser [no longer active]]

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]

Since this is really a bug in Core (gecko), and not in Firefox nor Camino, morphing this bug.

I found some more info from the WebKit people about what needs to be done to support this. This check-in is relevant: http://trac.webkit.org/projects/webkit/changeset/9274

The accessibility code probably just needs to support a few of those attributes and it should (in theory) work. With bug 352329 checked in, we'll be much closer.

Comment 19

[Håkan Waara]

*** 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)]

*** Bug 363340 has been marked as a duplicate of this bug. ***

Comment 25

[Patrick]

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]

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]

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)]

Are you sure this is a problem with Firefox?  Is Firefox still Carbon, or is it Cocoa yet? Bug 326469 is fixed, but I don't think that means it's completely Cocoa.  All my research seems to agree (not just this one blog post I'm linking), it's because Firefox isn't Cocoa: http://nixta.com/macswitch/?p=25

Comment 29

[[e-mail]]

I don't think being Cocoa or not has anything to do with this. As far as I know, BBEdit is still in Carbon and it does support dictionary lookup with cmd-ctrl-D key. One of my projects has a MLTE in a window and that control supports it automatically, too.

Comment 30

[[e-mail]]

Does this include Firefox, or is this just a Camino bug? My Firefox (Build 20090209020514) doesn't work with Cmd-Ctrl-D either.

Comment 31

[Samuel Sidler (old account; do not CC)]

This bug includes all Mozilla-based applications.

Comment 32

[Joe Drew (not getting mail)]

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)]

(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 [no longer active]]

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)]

Sounds like something for ChildView to do, then; thanks, smfr!  --> Widget:Cocoa

Comment 36

[Markus Stange [:mstange]]

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)]

what happened to this bug?

Comment 38

[Paul Borokhov (lensovet)]

also, it looks like NSTextInput is deprecated in 10.5 and above, and the NSTextInputClient protocol is its replacement.

Comment 39

[Markus Stange [:mstange]]

(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

[[e-mail]]

Someone that knows how please take this and fix it. The lack of Dictionary support has bugged me frequently because I use it regularly in other Mac OS X applications.

Thanks so much!

Comment 41

[Zac Zheng]

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)]

that's firefox-only; and you can use the services menu as well.
question for Marcus: if I wanted to look at implementing this, where exactly is the code? You mentioned a file name but no path to it...

Comment 43

[Markus Stange [:mstange]]

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)]

where can i get instructions on building?

Comment 45

[Markus Stange [:mstange]]

Here, for example:
https://developer.mozilla.org/En/Simple_Firefox_build

https://developer.mozilla.org/En/Participating_in_the_Mozilla_project is helpful, too.

Comment 46

[Nomis101]

(In reply to comment #42)
> that's firefox-only; 
It works also with Thunderbird 3.0, see release notes for Version 0.2.1

Comment 47

[Aaron Train [:aaronmt]]

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 49

[Markus Stange [:mstange]]

Attachment: wip

This is a rough start that only works in focused text fields.

Comment 50

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> -  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]]

(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]]

Attachment: 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]]

I've filed bug 569331 and bug 569334 on the focus and visual issues.

Comment 54

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 448502 [details] [diff] [review]
v1

Isn't the right result type (NSTextStrage*)?

http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSTextView_Class/Reference/Reference.html#//apple_ref/occ/instm/NSTextView/textStorage

Comment 55

[Markus Stange [:mstange]]

Indeed!

Comment 56

[Markus Stange [:mstange]]

Attachment: v2

with NSTextStorage instead of NSAttributedString

Comment 57

[José Jeria]

Will this land in time for Firefox 4? This was reviews back in July

Comment 58

[Markus Stange [:mstange]]

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]

The Firefox 4 RC1 is out and this feature is still broken.  It should be disabled as to not take up the cmd-ctrl-d hotkey until it is properly implemented.

I've mapped this hotkey to the 'look up in dictionary' service, and would prefer it to not conflict with a feature that does not even work consistently.

Comment 60

[Ricky Kung]

When will this feature be available? Firefox 6/7?
I checked Google Chrome Canary 14.0.808.0 just now and found that it has this feature already.

Comment 61

[Chris Lawson (gone)]

It'll be fixed when comment 58 is addressed. Please see

https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

Comment 63

[Xander S]

Am I correct in my assumption that development on this bug has halted due to 

://bugzilla.mozilla.org/show_bug.cgi?id=569331

And that development on that bug has halted due to time availability/technical issues?

Comment 64

[Chris Lawson (gone)]

(In reply to Xander S from comment #63)
> Am I correct in my assumption that development on this bug has halted due to 
> 
> bug 569331
> 
> And that development on that bug has halted due to time
> availability/technical issues?

That seems to be the case, yes.

Comment 66

[todor.dragnev]

2005 - 2013 - cmd+alt+D still not working :)

Comment 67

[Jared Wein [:jaws] (please needinfo? me)]

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]]

(In reply to Jared Wein [:jaws] from comment #67)
> Stephen, would you be willing to mentor this bug?

Sure. :-)

Comment 70

[nicolas.brouard]

Voting for this important feature. As well as for bug 569331, even if I don't understand if it is still related to or not.

Comment 72

[frankfang1990]

Vote for it.

Comment 73

[Marc Bejarano]

Jared: i guess some script took away all the bugs that were set Mentor:[e-mail]?  do you know of someone new to nominate as a mentor?  i ask since you originally nominated spohl.

BTW: Firefox 30.0 does allow the use of the keyboard shortcut in text entry fields

Comment 74

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

(In reply to Marc Bejarano from comment #73)
> Jared: i guess some script took away all the bugs that were set
> Mentor:[e-mail]?  do you know of someone new to nominate
> as a mentor?  i ask since you originally nominated spohl.
> 
> BTW: Firefox 30.0 does allow the use of the keyboard shortcut in text entry
> fields

There is now an explicit mentor field on each bug, that's all.  spohl is still marked down as the mentor for this bug.

Comment 75

[Stephen A Pohl [:spohl]]

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]]

Comment on attachment 456363 [details] [diff] [review]
v2

This no longer appears to be necessary.

Comment 77

[Stephen A Pohl [:spohl]]

(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+11]

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] (he/him)(JST, +0900)]

(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]]

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]]

(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]]

I have done the bug report. It would be much appreciated if :spohl or someone else can check and confirm it.

Comment 83

[Stephen A Pohl [:spohl]]

Removing myself as mentor here since this bug has become a meta bug.

Comment 84

[Jonathan Watt [:jwatt]]

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)

Comment 85

[Joshua Nixon]

Current build of Firefox (68.0b9 (64-bit)), cmd-ctrl-D shortcut opens dictionary definition every time. However, force-click on trackpad does NOT work - dictionary is not triggered. Seems like a regression, I was sure it worked in prior builds of Firefox.

Comment 86

[steve]

Firefox works fine with cmd ctrl D and Camino is EOL. Should this bug be closed?

Comment 87

[Stephen A Pohl [:spohl]]

Agreed. The majority of the issues related to this feature have been resolved. The few remaining open bugs in the dependent list can remain as follow up bugs and don't require this meta bug to remain open.