Bug 26832 - Generalize special height handling in InlineBox
Summary: Generalize special height handling in InlineBox
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 3749
  Show dependency treegraph
 
Reported: 2009-06-30 00:19 PDT by Roland Steiner
Modified: 2009-07-06 15:42 PDT (History)
1 user (show)

See Also:


Attachments
patch: implement proposed renaming (7.71 KB, patch)
2009-06-30 00:23 PDT, Roland Steiner
no flags Details | Formatted Diff | Diff
patch: renamed to 'virtualHeight' (7.40 KB, patch)
2009-06-30 23:22 PDT, Roland Steiner
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2009-06-30 00:19:47 PDT
InlineBox contains a bit that is used to mark when a box computes its height differently from the default. This bit is currently used only for SVG (and used instead of virtual methods for performance reasons) and hence named 'm_isSVG'.

This bit should have a generic name that fits its purpose so that it can be re-used.

The implementation for ruby will make use of this.
Comment 1 Roland Steiner 2009-06-30 00:22:22 PDT
(In reply to comment #0)

Addendum: This of course also applies to all methods that make use of this bit (isSVG, setIsSVG, svgBoxheight).

> InlineBox contains a bit that is used to mark when a box computes its height
> differently from the default. This bit is currently used only for SVG (and used
> instead of virtual methods for performance reasons) and hence named 'm_isSVG'.
> 
> This bit should have a generic name that fits its purpose so that it can be
> re-used.
> 
> The implementation for ruby will make use of this.
> 

Comment 2 Roland Steiner 2009-06-30 00:23:19 PDT
Created attachment 32036 [details]
patch: implement proposed renaming
Comment 3 Eric Seidel (no email) 2009-06-30 00:52:36 PDT
Comment on attachment 32036 [details]
patch: implement proposed renaming

It seems this should still be wrapped in ENABLE(SVG) || ENABLE(RUBY) blocks.

maybe m_useVirtualHeight would be more clear?  I'm not sure.  I'm just not sure what a "special height" is.
Comment 4 Roland Steiner 2009-06-30 23:22:45 PDT
Created attachment 32110 [details]
patch: renamed to 'virtualHeight'

You're right, 'virtualheight' probably is more clear.

Updated patch accordingly.
Comment 5 Maciej Stachowiak 2009-07-06 00:23:14 PDT
Comment on attachment 32110 [details]
patch: renamed to 'virtualHeight'

r=me
Comment 6 Roland Steiner 2009-07-06 04:11:45 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 15:42:44 PDT
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/rendering/InlineBox.cpp
	M	WebCore/rendering/InlineBox.h
	M	WebCore/rendering/RenderSVGInline.cpp
	M	WebCore/rendering/RenderSVGInlineText.cpp
	M	WebCore/rendering/RenderSVGText.cpp
	M	WebCore/rendering/SVGInlineFlowBox.h
	M	WebCore/rendering/SVGInlineTextBox.h
	M	WebCore/rendering/SVGRootInlineBox.h
Committed r45570
	M	WebCore/ChangeLog
	M	WebCore/rendering/InlineBox.cpp
	M	WebCore/rendering/RenderSVGInlineText.cpp
	M	WebCore/rendering/SVGInlineTextBox.h
	M	WebCore/rendering/SVGInlineFlowBox.h
	M	WebCore/rendering/RenderSVGText.cpp
	M	WebCore/rendering/RenderSVGInline.cpp
	M	WebCore/rendering/SVGRootInlineBox.h
	M	WebCore/rendering/InlineBox.h
r45570 = 0bc3a1356efe435f43de58095372a73b869cb918 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/45570