Closed Bug 718157 Opened 11 years ago Closed 8 years ago

Inconsistent layout with percentage text-indent

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: jruderman, Assigned: dbaron)

References

Details

(Keywords: testcase)

Attachments

(2 files)

Attached file 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
Blocks: refdyn
Depends on: 148396
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
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+
I think I do prefer it as two separate if statements.

https://hg.mozilla.org/integration/mozilla-inbound/rev/11e7ca322c9d
https://hg.mozilla.org/mozilla-central/rev/11e7ca322c9d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.