Summary: | Upwards cursor movement incorrect when previous block ends with <br> | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Julie Parent <jparent> | ||||||||||
Component: | HTML Editing | Assignee: | Ryosuke Niwa <rniwa> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, darin, dglazkov, enrica, justin.garcia, mitz, rniwa, tony, webkit.review.bot | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Windows XP | ||||||||||||
Attachments: |
|
Description
Julie Parent
2010-01-05 18:28:31 PST
Created attachment 48625 [details]
Patch
(In reply to comment #1) > Created an attachment (id=48625) [details] > Patch In theory I should have to do a similar check in nextLinePosition, but I couldn't come up with a test case where we have an inline element that takes up no space at the beginning of a block like a <br/> right before a </p>. Comment on attachment 48625 [details]
Patch
Maybe this check should move into a function with a nice name?
((o->isText() && toRenderText(o)->linesBoundingBox().height())
593 || (o->isBox() && toRenderBox(o)->borderBoundingBox().height())) {
like canHaveCursor(o)?
Created attachment 49179 [details]
Patch
Comment on attachment 49179 [details]
Patch
Good idea. Pulled out a function named canHaveCursor.
Comment on attachment 49179 [details]
Patch
OK.
Comment on attachment 49179 [details] Patch Clearing flags on attachment: 49179 Committed r55613: <http://trac.webkit.org/changeset/55613> All reviewed patches have been landed. Closing bug. This is still broken. To reproduce the bug, simply add <!DOCTYPE html> to the test added by r55613. Created attachment 103110 [details]
fixes the bug
Comment on attachment 103110 [details] fixes the bug Attachment 103110 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9306985 New failing tests: editing/execCommand/move-selection-back-line-rtl.html Created attachment 103120 [details]
rebaselined the test
Comment on attachment 103120 [details] rebaselined the test View in context: https://bugs.webkit.org/attachment.cgi?id=103120&action=review > LayoutTests/editing/execCommand/move-selection-back-line-rtl.html:6 > +<p>Move the caret from [ ] in the second line to [ ] in the first line.</p> It would be nice if this description were more specific like the old test description (what to do and what should happen). > LayoutTests/editing/execCommand/move-selection-back-line-strict.html:1 > +<div contentEditable="true" style="font-family: monospace;"> Did you mean to add a <!DOCTYPE html> here? Comment on attachment 103120 [details] rebaselined the test View in context: https://bugs.webkit.org/attachment.cgi?id=103120&action=review Thanks for the review. >> LayoutTests/editing/execCommand/move-selection-back-line-rtl.html:6 >> +<p>Move the caret from [ ] in the second line to [ ] in the first line.</p> > > It would be nice if this description were more specific like the old test description (what to do and what should happen). Will fix. >> LayoutTests/editing/execCommand/move-selection-back-line-strict.html:1 >> +<div contentEditable="true" style="font-family: monospace;"> > > Did you mean to add a <!DOCTYPE html> here? Huh that's odd. Apparently svn diff didn't pick up my change to this file. Will fix. Committed r92526: <http://trac.webkit.org/changeset/92526> |