1811311 - Consider Refactoring Various Baseline Functions

Status: RESOLVED FIXED

(Core :: Layout: Block and Inline, enhancement)

Description

[David Shin[:dshin]]

I see GetLogicalBaseline, GetVerticalAlignBaseline, GetNaturalBaselineBOffset, and BaselineBOffset.

The first 3 functions are virtual, the last function is not virtual but calls into GetNaturalBaselineBOffset. Through some hoops, all of these functions ultimately defer to others or use highly duplicated logic. See here for an example.

They make tracking how baselines get calculated tougher - ideally we should have one function for getting a baseline.

Some notes on each function:

GetLogicalBaseline:

• Always returns something (i.e. Will synthesize for you)
• Default nsIFrame function: Fallback synthesis
• Result relative to start of border edge
• Comment on how it's being replaced by GetVerticalAlignBaseline

GetVerticalAlignBaseline:

• Can return false (i.e. no baseline & no synthesis)
• Result relative to start of border edge

GetNaturalBaselineBOffset:

• Can return false (i.e. no baseline & no synthesis)
• Can select first/last baseline
• Result relative to start/end of border edge

BaselineBOffset:

• Always returns something (i.e. Will synthesize for you)
• Can select first/last baseline
• Result relative to start/end of border edge
• Calls GetNaturalBaselineBOffset

Comment 1

[David Shin[:dshin]]

Attachment: Bug 1811311 - Part 1: Unite baseline-related functions in `nsIFrame` into one. r=#layout-reviewers

Comment 2

[David Shin[:dshin]]

Attachment: Bug 1811311 - Part 2: Extract `SynthesizeBaselineBOffsetFrom*` out of `nsIFrame`. r=#layout-reviewers

These functions work fine as static functions, and nsIFrame is pretty overloaded.

Depends on D167990

Comment 3

[David Shin[:dshin]]

Attachment: Bug 1811311 - Part 3: Refactor `GetNaturalBaselineBOffset` to return `Maybe<nscoord>`. r=#layout-reviewers

Changed from returning bool & taking an out parameter.

Depends on D167991

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4846720fc2c6
Part 1: Unite baseline-related functions in `nsIFrame` into one. r=emilio
https://hg.mozilla.org/integration/autoland/rev/fbf495c37e23
Part 2: Extract `SynthesizeBaselineBOffsetFrom*` out of `nsIFrame`. r=emilio
https://hg.mozilla.org/integration/autoland/rev/ff31765de985
Part 3: Refactor `GetNaturalBaselineBOffset` to return `Maybe<nscoord>`. r=layout-reviewers,emilio

Comment 5

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/4846720fc2c6
https://hg.mozilla.org/mozilla-central/rev/fbf495c37e23
https://hg.mozilla.org/mozilla-central/rev/ff31765de985