Open Bug 1071098 Opened 10 years ago Updated 2 years ago

initial containing block is not copying "direction: rtl" from the root element (as observed via its abspos descendants' preference for honoring "right" vs. "left")

Categories

(Core :: Layout: Positioned, defect)

Product:
Core
Component:
Layout: Positioned
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
REOPENED

People

(Reporter: julienw, Assigned: dholbert)

References

(
URL
)

Details

Attachments

(4 files)

testcase 1
697 bytes, text/html
Details
reference case 1
323 bytes, text/html
Details
fix v1
8.11 KB, patch
dbaron
: review-
Details | Diff | Splinter Review
testcase showing propagation of direction and -webkit-writing-mode from <body> to the enclosing <html>
827 bytes, text/html
Details 
According to the working draft [1] and MDN [2], if both left, right, width are specified and we can't solve the problem, then we need to drop the "right" property if we're in LTR (we do) and drop the "left" property if we're in RTL (we don't).

See the [3] for a live example.

[1] http://dev.w3.org/csswg/css-position-3/#abs-non-replaced-width
[2] https://developer.mozilla.org/en-US/docs/Web/CSS/position#Notes
[3] http://jsbin.com/cozonetolaze/1/edit
In your example, the <html> element (the thing with dir="rtl") is *not* the containing block for the abspos thing.  Nor is the <body> element.

Rather, the *browser-viewport* (the parent of the <html>) is the abspos containing block.

If you add style="position:relative" to the <html> element or to the <body> element, then they'll become the abspos containing block, and you'll get the behavior you expect.

So, I think this is invalid.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
(In reply to Daniel Holbert [:dholbert] from comment #1)
> In your example, the <html> element (the thing with dir="rtl") is *not* the
> containing block for the abspos thing.  Nor is the <body> element.
> 
> Rather, the *browser-viewport* (the parent of the <html>) is the abspos
> containing block.

This is specified here:
  http://www.w3.org/TR/CSS21/visudet.html#containing-block-details

However, that spec text does say:
 "The 'direction' property of the initial containing block is the same as for the root element."

In your example, you aren't using the "direction" property, but if I tweak your testcase to do so, we still don't seem to honor that "direction" property, for elements positioned in the Initial Containing Block.  So, this might actually be buggy after all.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Attached file testcase 1 — 
Here's a testcase. (pretty similar to the original jsbin URL, but hosted on bugzilla and includes a spec reference)
Attached file reference case 1 — 
Here's a reference case for "testcase 1"; I've just added "position:relative" on the <html> element, to make *that* the abspos containing block for its descendants (instead of deferring to the initial containing block).

This produces expected results (a right-aligned div).

This comparison suggests that the problem is with the initial containing block not correctly taking "direction" from the root element.
Version: unspecified → Trunk
Summary: Absolute positioning does not follow RTL direction → initial containing block is not copying "direction: rtl" from the root element (as observed via its abspos children's preference for honoring "right" vs. "left")
Summary: initial containing block is not copying "direction: rtl" from the root element (as observed via its abspos children's preference for honoring "right" vs. "left") → initial containing block is not copying "direction: rtl" from the root element (as observed via its abspos descendants' preference for honoring "right" vs. "left")
Component: Layout: Block and Inline → Layout: R & A Pos
(In reply to Daniel Holbert [:dholbert] from comment #2)
> 
> In your example, you aren't using the "direction" property, but if I tweak
> your testcase to do so, we still don't seem to honor that "direction"
> property, for elements positioned in the Initial Containing Block.  So, this
> might actually be buggy after all.

I think "dir=rtl" uses the "direction" property under the hood (or at least everything looksas if it does).

Moreover, if the spec was specifying what you say in comment 1, we should definitely have changed the spec because it'd be confusing. I'm glad it doesn't :)

Thanks for the extra explanation!
(In reply to Julien Wajsberg [:julienw] from comment #5)
> I think "dir=rtl" uses the "direction" property under the hood (or at least
> everything looksas if it does).

(I think it does, too; I was just being careful to match the spec language exactly, for what gets copied where.)
Is this observable in any way other than via abspos/fixedpos?

If not, we should probably just fix that in the layout code, not by actually changing the styles of the initial containing block.

Specifically, in nsHTMLReflowState::InitAbsoluteConstraints where we do the two NS_STYLE_DIRECTION_RTL == cbrs->mStyleVisibility->mDirection checks.
(In reply to Boris Zbarsky [:bz] from comment #7)
> Is this observable in any way other than via abspos/fixedpos?

Nope, I'm not aware of any other way to inspect the "direction" property on the initial containing block.

> If not, we should probably just fix that in the layout code, not by actually
> changing the styles of the initial containing block.

Seems reasonable.  Should we check for this condition by checking whether cbrs->frame->GetType() is nsGkAtoms::canvasFrame? Or is there a better way to test "are we using the initial containing block" for this scenario?

(And then, I imagine we should use nsIDocument::GetRootElement()->GetPrimaryFrame()->StyleVisibility()->mDirection, as the "root element's 'direction' value")
(SIDE NOTE: Incidentally, Chrome and IE11 seem to allow <body style="direction:rtl"> to set the Initial Containing Block's "direction" property, too, even though <body> is not the root element.

I think this is a bug; I filed https://code.google.com/p/chromium/issues/detail?id=416600 on this in Blink, and depending on how that goes [i.e. if they say I'm misunderstanding the spec somehow], I'll probably file a similar bug for IE at some point.)
Assignee: nobody → dholbert
(In reply to Daniel Holbert [:dholbert] from comment #9)
> (SIDE NOTE: Incidentally, Chrome and IE11 seem to allow <body
> style="direction:rtl"> to set the Initial Containing Block's "direction"
> property, too, even though <body> is not the root element.
> 
> I think this is a bug; I filed
> https://code.google.com/p/chromium/issues/detail?id=416600 on this in Blink,
> and depending on how that goes [i.e. if they say I'm misunderstanding the
> spec somehow], I'll probably file a similar bug for IE at some point.)

In http://lists.w3.org/Archives/Public/www-style/2011Mar/0258.html the CSS WG explicitly resolved that it should not be propagated from body (see the resolution of issue 239).
Thanks! I noted that on the Blink bug, and I'll open an IE bug as well, in that case.
Attached patch fix v1 — — Splinter Review 
Here's a patch.

Some assumptions that I'm making in the patch:
 (1) The initial containing block can be detected by checking for GetType() == nsGkAtoms::canvasFrame. (Let me know if there's a better way to check for this)
 (2) The initial containing block's GetContent() method will give us the root element.
 (3) The root element's computed style can be obtained via its primary frame.

I've included two reftests -- one for <html> and one for <body>. The reftest files are all identical except for their first style rule.  If the CSS WG changes the specced behavior for <body> & "direction", we can adjust the second reftest when we implement that change.
Attachment #8493337 - Flags: review?(dbaron)
Comment on attachment 8493337 [details] [diff] [review]
fix v1

There are two places (at least) that do propagation of 'direction' to
the viewport.  One is in ScrollFrameHelper::IsLTR, and you're adding the
new one in ContainingBlockDirection.  They should at least have comments
pointing to each other, if not actually sharing code.

Also, given that IsLTR does propagation from the body, and that Chrome
and IE do for this case, I tend to think we probably should do so here
as well.  (And comment in the bugs filed in their bug systems.)

I think you also need to adjust
nsAbsoluteContainingBlock::FrameDependsOnContainer so that we don't try
to make resize reflow optimizations in cases where we can't now.

review- given these comments, I think

Also, it seems like we should have separate bugs on the following cases
not honoring the propagation of direction, although I think they're all
pretty low priority:

(1) The relative positioning case in nsHTMLReflowState::InitConstraints
    looks like it should be fixed; it affects what would happen to the root
    element when it's relatively positioned and has both 'left' and 'right'
    set.

(2) Likewise for nsHTMLReflowState::CalculateBlockSideMargins and the
    root element having overconstrained margins.

(3) Finally, the case of the root or body element being an absolutely
    positioned element (obscure, I think) whose position is determined
    using hypothetical box calculations (i.e., using auto offsets)  is
    wrong in two places, I think:

    First, nsHTMLReflowState::CalculateHypotheticalBox, and second, the
    placeholderFrame->GetContainingBlock()->StyleVisibility()->mDirection
    test in nsHTMLReflowState::InitAbsoluteConstraints.
Attachment #8493337 - Flags: review?(dbaron) → review-
What did you decide to do here?

I was about to align with IE's behavior (and I thought FF's behavior was the same, due to the initial scrolling position). Note that Chrome and IE does not do the same thing. (See https://code.google.com/p/chromium/issues/detail?id=416600 ).

The initial scroll position seems to be determined by the direction of <body> in Firefox (presumably because of ScrollFrameHelper::IsLTR), and the same in Chrome (even if I remove the propagation from <body>). Also in IE, of course, since it always propagates from <body>. Is this per spec? At least all browsers agree on this, but I would expect the initial scroll position to be determined by the same direction we propagate to the ICB.

DBaron, you suggest that Firefox should also propagate <body> to the ICB, because Chrome and IE does this. However, since Firefox is already not propagating direction from <body> (or <html> for that matter), we could probably just remove that behavior in Chrome---actually following the spec for once.

Or did you have other reasons for wanting body propagation?

Heh. In the initial patch that landed this in WebKit [1], the only reasons I find is that Firefox and IE does it.

[1] https://bugs.webkit.org/show_bug.cgi?id=48157
dbaron: ^
Flags: needinfo?(dbaron)
I'd rather not have direction propagated in different ways for different results (i.e., doing one thing for initial scroll position and another for abs pos).  That's complex for authors to understand (and to implement, and to spec).

Otherwise I don't think there are any decisions beyond what's visible in this bug.
Flags: needinfo?(dbaron)
(In reply to andersr from comment #17)
> The initial scroll position seems to be determined by the direction of
> <body> in Firefox (presumably because of ScrollFrameHelper::IsLTR), and the
> same in Chrome (even if I remove the propagation from <body>). Also in IE,
> of course, since it always propagates from <body>. Is this per spec? At
> least all browsers agree on this, but I would expect the initial scroll
> position to be determined by the same direction we propagate to the ICB.

Er, and I don't think the spec specifies which direction scrolling is allowed in (which I think is a better way of explaining it than "initial scroll position").  But I agree with you expectation that it should be determined by the same direction that is propagated to the ICB.
Related to this, although distinct from the question of propagating to the ICB, is the question of whether direction and/or writing-mode properties should "magically" propagate from the <body> up to the root <html> element (where they could be observed by getComputedStyle(), in addition to propagating from there to the ICB). Currently, it looks like webkit does this, but gecko doesn't. Should we?
Flags: needinfo?(dbaron)
This appears to show that Webkit propagates these properties up from the <body> (unless they're explicitly specified on <html>), while Gecko doesn't.
(In reply to Jonathan Kew (:jfkthame) from comment #22)
> Related to this, although distinct from the question of propagating to the
> ICB, is the question of whether direction and/or writing-mode properties
> should "magically" propagate from the <body> up to the root <html> element
> (where they could be observed by getComputedStyle(), in addition to
> propagating from there to the ICB). Currently, it looks like webkit does
> this, but gecko doesn't. Should we?

If IE also does it for 'direction', then I think we should probably just switch to doing it for both 'direction' and 'writing-mode', and push the CSS WG to change (we considered it a few years back).

If it's just WebKit, then I'd probably be ok with going either way, although the path of least resistance may well be to do the same (just as we do for 'background-*', and (somewhat inconsistently) for 'overflow').
Flags: needinfo?(dbaron)
Bug 1161752 was, for all practical purposes, a duplicate of this bug report; I just did not find it.

(In reply to Daniel Holbert [:dholbert] from comment #6)
> > I think "dir=rtl" uses the "direction" property under the hood (or at least
> > everything looksas if it does).
> 
> (I think it does, too; I was just being careful to match the spec language
> exactly, for what gets copied where.)

"
6.4.4 Precedence of non-CSS presentational hints
The UA may choose to honor presentational attributes in an HTML source document. If so, these attributes are translated to the corresponding CSS rules with specificity equal to 0, and are treated as if they were inserted at the start of the author style sheet.
(...)
For HTML, any attribute that is not in the following list should be considered presentational: (...) dir, 
"
http://www.w3.org/TR/CSS21/cascade.html#preshint

but this must be a spec mistake as 'dir' attribute is definitely a presentational attribute.
(In reply to Gérard Talbot from comment #26)
> For HTML, any attribute that is not in the following list should be
> considered presentational: (...) dir, 
> "
> http://www.w3.org/TR/CSS21/cascade.html#preshint
> 
> but this must be a spec mistake as 'dir' attribute is definitely a
> presentational attribute.

The 'dir' attribute is definitely not a presentational attribute, any more than the 'lang' attribute, which is why both HTML5 and CSS3 Writing Modes recommend that authors use it in preference to CSS 'direction'. I'm not sure if 'dir' using 'direction' under the hood is specified anywhere in the prose of HTML or CSS, but it's recommended in https://html.spec.whatwg.org/multipage/rendering.html#bidi-rendering and http://www.w3.org/TR/CSS2/sample.html
Also see bug 1169065 about propagation from body to root.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: