Bug 26829

Summary: add methods baselinePosition and lineHeight to InlineBox
Product: WebKit Reporter: Roland Steiner <rolandsteiner>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Trivial CC: bfulgham, eric
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 3749    
Attachments:
Description Flags
patch: add methods lineHeight and baselinePosition to InlineBox
mjs: review-
patch: removed unrelad
none
patch: removed unrelated code change from initial patch
eric: review-
patch: update to remove conflicts
none
patch: remove tabs
none
patch: remove tabs AND spurious file references eric: review+

Description Roland Steiner 2009-06-29 22:55:27 PDT
[outtake from larger ruby patch - see bug #3749]

There are quite a few places in the code of InlineBox and InlineFlowBox in the form:

boxPtr->renderer()->lineHeight()

same for baselinePosition(). Adding corresponding methods to InlineBox would make the code more readable.

Also, implementation for ruby will make use of those new methods.
Comment 1 Roland Steiner 2009-06-29 22:56:32 PDT
Created attachment 32033 [details]
patch: add methods lineHeight and baselinePosition to InlineBox
Comment 2 Maciej Stachowiak 2009-07-06 00:29:23 PDT
Comment on attachment 32033 [details]
patch: add methods lineHeight and baselinePosition to InlineBox

There is a chunk in here that's unrelated to the refactoring mentioned in the ChangeLog. I discussed with Roland, and he is going to submit a new patch.
Comment 3 Roland Steiner 2009-07-06 01:26:09 PDT
Created attachment 32288 [details]
patch: removed unrelad
Comment 4 Roland Steiner 2009-07-06 01:27:42 PDT
Created attachment 32289 [details]
patch: removed unrelated code change from initial patch
Comment 5 Maciej Stachowiak 2009-07-06 01:37:59 PDT
Comment on attachment 32289 [details]
patch: removed unrelated code change from initial patch

r=me
Comment 6 Roland Steiner 2009-07-06 04:10:48 PDT
(In reply to comment #5)

Thanks a lot for the review! Can I ask you to commit the patch for me, please?
Comment 7 Eric Seidel (no email) 2009-07-06 16:30:53 PDT
Comment on attachment 32289 [details]
patch: removed unrelated code change from initial patch

patching file WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/rendering/InlineBox.h
Hunk #1 succeeded at 205 (offset 4 lines).
Hunk #2 FAILED at 283.
1 out of 2 hunks FAILED -- saving rejects to file WebCore/rendering/InlineBox.h.rej
patch -p0 "WebCore/rendering/InlineBox.h" returned 1.  Pass --force to ignore patch failures.
Patch https://bugs.webkit.org/attachment.cgi?id=32289 failed to download and apply.
Comment 8 Eric Seidel (no email) 2009-07-06 16:31:21 PDT
Someone could possibly land this manually.  I didn't investigate the failure.
Comment 9 Roland Steiner 2009-07-06 19:34:33 PDT
Created attachment 32352 [details]
patch: update to remove conflicts

Same as previous patch, updated to remove patching conflicts
Comment 10 Eric Seidel (no email) 2009-07-06 19:37:50 PDT
Comment on attachment 32352 [details]
patch: update to remove conflicts

It looks like WebCore/rendering/InlineBox.h has tabs. :(
Comment 11 Eric Seidel (no email) 2009-07-06 19:38:39 PDT
Comment on attachment 32352 [details]
patch: update to remove conflicts

Our tooling infrastructure should really warn you about that kind of thing.  Otherwise looks fine.
Comment 12 Roland Steiner 2009-07-06 20:21:59 PDT
Created attachment 32356 [details]
patch: remove tabs

@_@ How did that happen? Updated patch to remove tabs.
Comment 13 Roland Steiner 2009-07-06 20:29:39 PDT
Created attachment 32357 [details]
patch: remove tabs AND spurious file references

Today seems to be not my day.  >_< Removed spurious file references at the end of the patch file...

Sorry for the attachment spam.
Comment 14 Eric Seidel (no email) 2009-07-06 20:32:08 PDT
Comment on attachment 32357 [details]
patch: remove tabs AND spurious file references

Looks fine.
Comment 15 Roland Steiner 2009-07-08 21:27:57 PDT
(In reply to comment #14)

Would still need a kind soul to commit this for me, though... ^_^;
Comment 16 Brent Fulgham 2009-07-09 22:13:57 PDT
Landed in http://trac.webkit.org/changeset/45701