Bug 115256 - Use the complete block as the context for Dictionary lookups
Summary: Use the complete block as the context for Dictionary lookups
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-04-26 06:35 PDT by Thomas Deniau
Modified: 2013-05-30 01:28 PDT (History)
7 users (show)

See Also:


Attachments
Patch (6.04 KB, patch)
2013-04-26 06:47 PDT, Thomas Deniau
no flags Details | Formatted Diff | Diff
Patch (9.98 KB, patch)
2013-05-23 08:21 PDT, Thomas Deniau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Deniau 2013-04-26 06:35:03 PDT
Use the complete block as the context for Dictionary lookups
Comment 1 Thomas Deniau 2013-04-26 06:47:34 PDT
Created attachment 199826 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2013-04-26 06:50:06 PDT
<rdar://problem/13747883>
Comment 3 Thomas Deniau 2013-04-26 06:52:23 PDT
When computing the context we give when looking up a word in the Dictionary, use startOfBlock/endOfBlock instead of startOfParagraph/endOfParagraph so that the context doesn't stop at line breaks.
Comment 4 Ryosuke Niwa 2013-04-26 11:39:32 PDT
Comment on attachment 199826 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=199826&action=review

> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:510
> +    // A "paragraph" is not enough as it may stop at a <br> and Lookup sometimes needs several lines of context.

If paragraph isn’t good enough, then block is probably not good enough either.
You realize that we can have each paragraph being isolated by a div, right?
Comment 5 Thomas Deniau 2013-04-26 12:35:54 PDT
Comment on attachment 199826 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=199826&action=review

>> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:510
>> +    // A "paragraph" is not enough as it may stop at a <br> and Lookup sometimes needs several lines of context.
> 
> If paragraph isn’t good enough, then block is probably not good enough either.
> You realize that we can have each paragraph being isolated by a div, right?

I do. This is of course best-effort and there's no solution that can work with all the web pages out there. My take is that a good Web author will put any related information (that Lookup cares about) in the same block. If that doesn't work, Lookup will do its best given the partial context. The user can select text manually if that's not enough...
Comment 6 Ryosuke Niwa 2013-04-26 13:02:43 PDT
Comment on attachment 199826 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=199826&action=review

>>> Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:510
>>> +    // A "paragraph" is not enough as it may stop at a <br> and Lookup sometimes needs several lines of context.
>> 
>> If paragraph isn’t good enough, then block is probably not good enough either.
>> You realize that we can have each paragraph being isolated by a div, right?
> 
> I do. This is of course best-effort and there's no solution that can work with all the web pages out there. My take is that a good Web author will put any related information (that Lookup cares about) in the same block. If that doesn't work, Lookup will do its best given the partial context. The user can select text manually if that's not enough...

Why don’t we get 3 paragraphs around the word instead?
Comment 7 Thomas Deniau 2013-05-23 08:21:58 PDT
Created attachment 202717 [details]
Patch
Comment 8 WebKit Commit Bot 2013-05-24 12:41:17 PDT
Comment on attachment 202717 [details]
Patch

Clearing flags on attachment: 202717

Committed r150653: <http://trac.webkit.org/changeset/150653>
Comment 9 WebKit Commit Bot 2013-05-24 12:41:19 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Tim Horton 2013-05-30 01:28:35 PDT
This caused https://bugs.webkit.org/show_bug.cgi?id=117020.