Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Renaming things

33 views
Skip to first unread message

L. David Baron

unread,
Jul 21, 2013, 12:16:47 AM7/21/13
to dev-tec...@lists.mozilla.org
When we met in Taipei we had some discussion about how there were
some code that was unnecessarily confusing due to poor names. I
collected the following list of renames then (and since) that I'd
like us to make. The list I've collected so far is below?
Thoughts? Additions? Opinions on the options below for
block-in-inline splits?

-David

nsHTMLReflowState -> mozilla::ReflowInput

nsHTMLReflowMetrics -> mozilla::ReflowOutput

nsHTMLReflowState::availableHeight -> nsHTMLReflowState::distanceToBreak

NS_FRAME_IS_SPECIAL -> NS_FRAME_BLOCK_IN_INLINE_SPLIT or NS_FRAME_IB_SPLIT
IsFrameSpecial -> IsFramePartOfBlockInInlineSplit or ...IBSplit
etc.
(opinions on whether the shorter or longer names are better?)

nsHTMLCSSStyleSheet -> mozilla::StyleAttributeRuleProcessor
nsHTMLStyleSheet -> mozilla::PresentationAttributeRuleProcessor
(the first step here was https://bugzilla.mozilla.org/show_bug.cgi?id=882573)

(Although I also wonder if we should rename RuleProcessor to
something like RuleSource or RuleProvider.)

nsGfxScrollFrameInner -> mozilla::GfxScrollFrameHelper
(This is confusing because there is an inner frame too!)

removing mozilla::css namespace and just having the things in it be
in mozilla, in some cases with class names beginning with CSS.

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla http://www.mozilla.org/ 𝄂

Robert O'Callahan

unread,
Jul 21, 2013, 1:18:39 AM7/21/13
to L. David Baron, dev-tec...@lists.mozilla.org
On Sun, Jul 21, 2013 at 4:16 PM, L. David Baron <dba...@dbaron.org> wrote:

> nsHTMLReflowState::availableHeight -> nsHTMLReflowState::distanceToBreak
>

Maybe I'm captured, but availableHeight actually sounds better to me.


> NS_FRAME_IS_SPECIAL -> NS_FRAME_BLOCK_IN_INLINE_SPLIT or NS_FRAME_IB_SPLIT
> IsFrameSpecial -> IsFramePartOfBlockInInlineSplit or ...IBSplit
> etc.
> (opinions on whether the shorter or longer names are better?)
>

IBSplit is better IMHO. Although maybe BISplit is even better, since it's
"Block in Inline".

nsGfxScrollFrameInner -> mozilla::GfxScrollFrameHelper
> (This is confusing because there is an inner frame too!)
>

Just mozilla::ScrollFrameHelper.


> removing mozilla::css namespace and just having the things in it be
> in mozilla, in some cases with class names beginning with CSS.
>

Sounds good. What about mozilla::layout?

Rob
--
Jtehsauts tshaei dS,o n" Wohfy Mdaon yhoaus eanuttehrotraiitny eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.r"t sS?o Whhei csha iids teoa
stiheer :p atroa lsyazye,d 'mYaonu,r "sGients uapr,e tfaokreg iyvoeunr,
'm aotr atnod sgaoy ,h o'mGee.t" uTph eann dt hwea lmka'n? gBoutt uIp
waanndt wyeonut thoo mken.o w *
*

Robert O'Callahan

unread,
Jul 21, 2013, 1:19:57 AM7/21/13
to L. David Baron, dev-tec...@lists.mozilla.org
On Sun, Jul 21, 2013 at 5:18 PM, Robert O'Callahan <rob...@ocallahan.org>wrote:

> On Sun, Jul 21, 2013 at 4:16 PM, L. David Baron <dba...@dbaron.org> wrote:
>
> NS_FRAME_IS_SPECIAL -> NS_FRAME_BLOCK_IN_INLINE_SPLIT or NS_FRAME_IB_SPLIT
>> IsFrameSpecial -> IsFramePartOfBlockInInlineSplit or ...IBSplit
>> etc.
>> (opinions on whether the shorter or longer names are better?)
>>
>
> IBSplit is better IMHO. Although maybe BISplit is even better, since it's
> "Block in Inline".
>

Actually I changed my mind already. IsFramePartOfBlockInInlineSplit is just
clearer.

L. David Baron

unread,
Jul 21, 2013, 1:33:25 AM7/21/13
to Robert O'Callahan, dev-tec...@lists.mozilla.org
On Sunday 2013-07-21 17:18 +1200, Robert O'Callahan wrote:
> On Sun, Jul 21, 2013 at 4:16 PM, L. David Baron <dba...@dbaron.org> wrote:
>
> > nsHTMLReflowState::availableHeight -> nsHTMLReflowState::distanceToBreak
> >
>
> Maybe I'm captured, but availableHeight actually sounds better to me.

At least part of the problem is that availableHeight and
availableWidth are totally different things, which isn't what it
sounds like from the name.

> > removing mozilla::css namespace and just having the things in it be
> > in mozilla, in some cases with class names beginning with CSS.
> >
>
> Sounds good. What about mozilla::layout?

Hmmm. Not that much in it, but probably the same? At first glance,
the things in it seem to have unique enough names to be in mozilla.
(I don't know anything about the ipc stuff, though.)

-David

Robert O'Callahan

unread,
Jul 21, 2013, 1:49:43 AM7/21/13
to L. David Baron, dev-tec...@lists.mozilla.org
On Sun, Jul 21, 2013 at 5:33 PM, L. David Baron <dba...@dbaron.org> wrote:

> At least part of the problem is that availableHeight and
> availableWidth are totally different things, which isn't what it
> sounds like from the name.
>

Really? availableWidth is the distance to the next horizontal break, and
availableHeight is the distance to the next vertical break. No?

Cameron McCormack

unread,
Jul 21, 2013, 2:20:11 AM7/21/13
to L. David Baron, dev-tec...@lists.mozilla.org
Do nsFrameManagerBase, nsFrameManager and nsCSSFrameConstructor still
need to be separate classes?

Any reason not to rename nsTextFrameThebes.cpp to nsTextFrame.cpp?

L. David Baron

unread,
Jul 27, 2013, 5:11:42 PM7/27/13
to Cameron McCormack, dev-tec...@lists.mozilla.org
On Sunday 2013-07-21 16:20 +1000, Cameron McCormack wrote:
> Do nsFrameManagerBase, nsFrameManager and nsCSSFrameConstructor
> still need to be separate classes?

The first two, probably not. The third, maybe, if only to keep
distinct code separate.

> Any reason not to rename nsTextFrameThebes.cpp to nsTextFrame.cpp?

No, we've been talking about it for years, along with a list of
other file moves. I should probably do that and moving
nsStyleConsts.h into layout/style/ ASAP.



Another thought I had was maybe:
nsIStyleRule -> mozilla::SpecifiedStyle
nsIStyleRuleProcessor -> mozilla::SpecifiedStyleSource
though I'm far from sold on it yet.


Also, I realize that we've had some confusion from the XUL layout
methods, originally separate on nsIBox, that have been merged into
nsIFrame. Perhaps those should have a XUL prefix (e.g.,
XULLayout(), XULBorderAndPadding(), etc.). See the big chunk of
nsIFrame.h between "BOX LAYOUT METHODS" and "END OF BOX LAYOUT
METHODS".

Ehsan Akhgari

unread,
Jul 28, 2013, 12:05:12 PM7/28/13
to L. David Baron, Cameron McCormack, dev-tec...@lists.mozilla.org
On Sat, Jul 27, 2013 at 5:11 PM, L. David Baron <dba...@dbaron.org> wrote:

> Also, I realize that we've had some confusion from the XUL layout
> methods, originally separate on nsIBox, that have been merged into
> nsIFrame. Perhaps those should have a XUL prefix (e.g.,
> XULLayout(), XULBorderAndPadding(), etc.). See the big chunk of
> nsIFrame.h between "BOX LAYOUT METHODS" and "END OF BOX LAYOUT
> METHODS".


That's a great idea!

--
Ehsan
<http://ehsanakhgari.org/>

L. David Baron

unread,
Jul 28, 2013, 3:41:53 PM7/28/13
to dev-tec...@lists.mozilla.org
One more item for the renaming list:

We should rename nsTableOuterFrame to nsTableWrapperFrame, since the
CSS spec has (since nsTableOuterFrame was written) created a term
(table wrapper box) for the thing implemented by nsTableOuterFrame:
http://www.w3.org/TR/CSS21/tables.html#model
0 new messages