Inconsistent layout with percentage text-indent

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: jruderman, Assigned: dbaron)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla38
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

7 years ago
Created attachment 588603 [details]
testcase

1. Load the testcase
2. Click the first button, "Widen the outer box"
3. Click the second button, "Force re-layout"

Result: The 'X' moves at step 3.

Expected: I don't know what should happen at step 2, but step 3 should have no effect.

(Why is the text-indent working as a percentage of the outer box's width, rather than a percentage of the inner box's width?)
> Why is the text-indent working as a percentage of the outer box's width

Because the spec says so.  http://www.w3.org/TR/CSS21/text.html#indentation-prop says:

  Percentages: refer to width of containing block 

Amusingly enough, Opera has this bug too.

I wonder whether we're managing to optimize away the inner block's reflow entirely or something....
Component: Layout → Layout: Block and Inline
QA Contact: layout → layout.block-and-inline
(Reporter)

Updated

7 years ago
Blocks: 373610
No longer depends on: 148396
Not sure why bug 45631's patch didn't fix this.
OS: Mac OS X → All
Hardware: x86_64 → All
(Er, probably because of what bz said at the end of comment 1.)
Well, in particular, nsBlockFrame::Reflow does:

  if (!(GetStateBits() & NS_FRAME_IS_DIRTY) && reflowState->IsIResize()) {
    PrepareResizeReflow(state);
  }

so we don't even call PrepareResizeReflow if only the parent resized.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Created attachment 8562537 [details] [diff] [review]
Fix dynamic changes of percentage text-indent when the parent's size changes and the element's size does not

This code needs to move out of PrepareResizeReflow() (and things that
trigger code in it) because PrepareResizeReflow is conditioned on the
block itself resizing.

The reftest is based on a test by Jesse Ruderman <jruderman@gmail.com>.

The reftest fails without the patch and passes with the patch.
Attachment #8562537 - Flags: review?(jfkthame)
Comment on attachment 8562537 [details] [diff] [review]
Fix dynamic changes of percentage text-indent when the parent's size changes and the element's size does not

Review of attachment 8562537 [details] [diff] [review]:
-----------------------------------------------------------------

Patch looks fine, given the current spec, although in this case I think the spec is bad. But that's orthogonal to fixing the dynamic-change bug.

::: layout/generic/nsBlockFrame.cpp
@@ +1148,5 @@
> +      reflowState->mCBReflowState->IsIResize() &&
> +      reflowState->mStyleText->mTextIndent.HasPercent() &&
> +      !mLines.empty()) {
> +    mLines.front()->MarkDirty();
> +  }

You could put both this and the preceding statement into a shared NS_FRAME_IS_DIRTY condition, to avoid the repeated testing of that flag. Though I expect the difference is insignificant, if you prefer the readability of this version.
Attachment #8562537 - Flags: review?(jfkthame) → review+
https://hg.mozilla.org/mozilla-central/rev/11e7ca322c9d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.