Bug 118447

Summary: Range.getClientRects() not working correctly for partially contained vertically styled text nodes
Product: WebKit Reporter: Sam Weinig <sam>
Component: Layout and RenderingAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, enrica, esprehn+autocc, glenn, hyatt, simon.fraser
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Test Case
none
Potential fix from Mac Murrett
none
Patch hyatt: review+, sam: commit-queue+

Sam Weinig
Reported 2013-07-06 20:50:56 PDT
Created attachment 206197 [details] Test Case Range.getClientRects() is not working correctly for partially contained vertically styled text nodes. (See attached test case).
Attachments
Test Case (1.32 KB, text/html)
2013-07-06 20:50 PDT, Sam Weinig
no flags
Potential fix from Mac Murrett (527 bytes, patch)
2013-07-10 10:44 PDT, Sam Weinig
no flags
Patch (4.76 KB, patch)
2013-07-10 11:59 PDT, Sam Weinig
hyatt: review+
sam: commit-queue+
Sam Weinig
Comment 1 2013-07-10 10:44:21 PDT
Created attachment 206397 [details] Potential fix from Mac Murrett
Dave Hyatt
Comment 2 2013-07-10 11:15:30 PDT
Comment on attachment 206397 [details] Potential fix from Mac Murrett View in context: https://bugs.webkit.org/attachment.cgi?id=206397&action=review > Source/WebCore/rendering/RenderText.cpp:338 > r.setHeight(box->logicalHeight()); > r.setY(box->y()); > } else { > - r.setWidth(box->logicalWidth()); > + r.setWidth(box->logicalHeight()); > r.setX(box->x()); Shouldn't this just be using box->width() and box->height()? Seems silly to call logical when you seem to want physical?
Sam Weinig
Comment 3 2013-07-10 11:16:30 PDT
(In reply to comment #2) > (From update of attachment 206397 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206397&action=review > > > Source/WebCore/rendering/RenderText.cpp:338 > > r.setHeight(box->logicalHeight()); > > r.setY(box->y()); > > } else { > > - r.setWidth(box->logicalWidth()); > > + r.setWidth(box->logicalHeight()); > > r.setX(box->x()); > > Shouldn't this just be using box->width() and box->height()? Seems silly to call logical when you seem to want physical? Agreed, I'll try that out.
Sam Weinig
Comment 4 2013-07-10 11:59:42 PDT
Dave Hyatt
Comment 5 2013-07-10 12:00:56 PDT
Comment on attachment 206404 [details] Patch r=me
Sam Weinig
Comment 6 2013-07-10 16:57:36 PDT
Daniel Bates
Comment 7 2013-07-10 17:42:34 PDT
Note You need to log in before you can comment on or make changes to this bug.