Closed Bug 1588017 Opened 3 months ago Closed 2 months ago

Clean up the naming for IntrinsicISizeOffsetData

Categories

(Core :: Layout, task, P5)

task

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: TYLin, Assigned: alaskanemily)

Details

Attachments

(1 file)

The IntrinsicISizeOffsetData is defined as follows.

  struct IntrinsicISizeOffsetData {
    nscoord hPadding, hBorder, hMargin;

    IntrinsicISizeOffsetData() : hPadding(0), hBorder(0), hMargin(0) {}
  };

It is used as the return type by both IntrinsicISizeOffsets and IntrinsicBSizeOffsets.

So it might be better to

  1. Change the name to IntrinsicSizeOffsetData by replacing ISize as Size
  2. Get rid of the 'h'-prefix in the fields.

We should also clean up the code-comments around this type & associated functions.
Code snippet:
https://searchfox.org/mozilla-central/rev/1fe0cf575841dbf3b7e159e88ba03260cd1354c0/layout/generic/nsIFrame.h#2360-2381

The first comment there wants to move down to be adjacent to the function decl rather than the type.

And the type should get its own new short comment that refers to the functions below it & that (importantly) indicates that it represents margin/border/padding intrinsic-size-data for a single axis. (In our type names, the word "size" usually means a 2-dimensional size. Here, the "I" in "ISize" originally indicated that it was single-axis; so now that we'll be removing the "I", we'll want to add a brief note to make it clear that this is indeed a single-axis thing.)

(And in that above-linked code snippet, "horizontal" should be replaced with "isize" - that "horizontal" word is a relic from before we supported writing-modes.)

Assignee: nobody → emcdonough
Status: NEW → ASSIGNED

Update the comments, name, and fields to show it is agnostic of isize/bsize.

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/189852a9cf2a
Clean up nsIFrame::IntrinsicISizeOffsetData r=TYLin,dholbert
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
You need to log in before you can comment on or make changes to this bug.