Closed Bug 1319672 Opened 3 years ago Closed 3 years ago

Improve document related to writing-mode in nsFloatManager

Categories

(Core :: Layout: Floats, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(2 files)

Per bug 1316549 comment 9, we should document aWM, mWritingMode, and perhaps CHECK_BLOCK_DIR usage in nsFloatManager.
Depends on: 1316549
Priority: -- → P3
Comment on attachment 8813980 [details]
Bug 1319672 Part 1 - Add document related to writing-mode in nsFloatManager.

https://reviewboard.mozilla.org/r/95284/#review96236

::: layout/generic/nsFloatManager.h:48
(Diff revision 1)
>      , mHasFloats(aHasFloats) {}
>  };
>  
>  #define NS_FLOAT_MANAGER_CACHE_SIZE 4
>  
> +/**

Given that this is the comment describing the entire class, you should probably start with an additional first paragraph like:

nsFloatManager is responsible for implementing CSS's rules for positioning floats.  An nsFloatManager object is created during reflow for any block with NS_BLOCK_FLOAT_MGR.  During reflow, the float manager for the nearest such ancestor block is found in ReflowInput::mFloatManager.

::: layout/generic/nsFloatManager.h:49
(Diff revision 1)
>  };
>  
>  #define NS_FLOAT_MANAGER_CACHE_SIZE 4
>  
> +/**
> + * According to CSS Writing Modes Level 3 §7.5, line-right and line-left

Please link to https://drafts.csswg.org/css-writing-modes/#line-mappings rather than giving section numbers, since section numbers change.

::: layout/generic/nsFloatManager.h:50
(Diff revision 1)
>  
>  #define NS_FLOAT_MANAGER_CACHE_SIZE 4
>  
> +/**
> + * According to CSS Writing Modes Level 3 §7.5, line-right and line-left
> + * are calculated with respect to the writing-mode of the containing block

Throughout, this should be "writing mode" (the concept) rather than "writing-mode" (the CSS property).

::: layout/generic/nsFloatManager.h:51
(Diff revision 1)
>  #define NS_FLOAT_MANAGER_CACHE_SIZE 4
>  
> +/**
> + * According to CSS Writing Modes Level 3 §7.5, line-right and line-left
> + * are calculated with respect to the writing-mode of the containing block
> + * of the float element. All the writing-mode pass to nsFloatManager methods

"float element" -> "float"

::: layout/generic/nsFloatManager.h:51
(Diff revision 1)
>  #define NS_FLOAT_MANAGER_CACHE_SIZE 4
>  
> +/**
> + * According to CSS Writing Modes Level 3 §7.5, line-right and line-left
> + * are calculated with respect to the writing-mode of the containing block
> + * of the float element. All the writing-mode pass to nsFloatManager methods

"writing-mode pass" -> "writing modes passed"

::: layout/generic/nsFloatManager.h:54
(Diff revision 1)
> + * According to CSS Writing Modes Level 3 §7.5, line-right and line-left
> + * are calculated with respect to the writing-mode of the containing block
> + * of the float element. All the writing-mode pass to nsFloatManager methods
> + * should be the containing block's writing-mode.
> + *
> + * However, according to the abstract-to-physical mappings table in §6.4

Again, use a URL rather than a section number.

::: layout/generic/nsFloatManager.h:56
(Diff revision 1)
> + * interpretation of line-right and line-left. We actually implemented as
> + * passing in the writing-mode of the block formatting context (BFC), i.e.

"implemented as passing" -> "implement this by passing"

::: layout/generic/nsFloatManager.h:400
(Diff revision 1)
> +  // Store the WritingMode of ReflowInput from the block frame which
> +  // established the block formatting context (BFC) when nsFloatManager is
> +  // created.

I'm not sure what the ReflowInput has to do with this, so drop "of ReflowInput".  Also, "established" -> "establishes" and "when nsFloatManager" -> "when the nsFloatManager".
Attachment #8813980 - Flags: review?(dbaron) → review+
Comment on attachment 8813981 [details]
Bug 1319672 Part 2 - Check the line direction is not changed.

https://reviewboard.mozilla.org/r/95286/#review96240
Attachment #8813981 - Flags: review?(dbaron) → review+
All the issues in comment 3 have been addressed in patchset 2.
Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/81eb4fbbb3ef
Part 1 - Add document related to writing-mode in nsFloatManager. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/cb5869f00c9f
Part 2 - Check the line direction is not changed. r=dbaron
https://hg.mozilla.org/mozilla-central/rev/81eb4fbbb3ef
https://hg.mozilla.org/mozilla-central/rev/cb5869f00c9f
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.