Bug 115256

Summary: Use the complete block as the context for Dictionary lookups
Product: WebKit Reporter: Thomas Deniau <deniau>
Component: HTML EditingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, enrica, graouts, jiapu.mail, rniwa, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Thomas Deniau
Reported 2013-04-26 06:35:03 PDT
Use the complete block as the context for Dictionary lookups
Attachments
Patch (6.04 KB, patch)
2013-04-26 06:47 PDT, Thomas Deniau 		no flags
DetailsFormatted DiffDiff
Patch (9.98 KB, patch)
2013-05-23 08:21 PDT, Thomas Deniau 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Thomas Deniau
Comment 1 2013-04-26 06:47:34 PDT
Created attachment 199826 [details] Patch
Radar WebKit Bug Importer
Comment 2 2013-04-26 06:50:06 PDT
<rdar://problem/13747883>
Thomas Deniau
Comment 3 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.
Ryosuke Niwa
Comment 4 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?
Thomas Deniau
Comment 5 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...
Ryosuke Niwa
Comment 6 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?
Thomas Deniau
Comment 7 2013-05-23 08:21:58 PDT
Created attachment 202717 [details] Patch
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2013-05-24 12:41:19 PDT
All reviewed patches have been landed. Closing bug.
Tim Horton
Comment 10 2013-05-30 01:28:35 PDT
This caused https://bugs.webkit.org/show_bug.cgi?id=117020.
Note You need to log in before you can comment on or make changes to this bug.