bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

need logical-direction equivalents of the CSS width and height properties

RESOLVED FIXED in mozilla38

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: jfkthame, Assigned: heycam)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

Trunk
mozilla38
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
In addition to logical versions of the box-edge-related properties (margins, borders, padding) in bug 1083134, we'll also need logical versions of the width and height properties, and their min- and max- counterparts.

This is needed to make it possible to write stylesheets such as html.css, ua.css, forms.css, etc., in a form that works for both horizontal and vertical writing modes.

:heycam, any chance you could handle this along with the work you're already doing in bug 1083134?
Version: unspecified → Trunk
(Assignee)

Comment 1

4 years ago
This is offset-{block,inline}-{start,end}?  Yes, I can take a look at that.

One issue: my patches in bug 1083134 assume that logical longhand properties all have a corresponding 4-component physical shorthand they correspond to.  For example, margin-inline-start is associated with margin.  This is used in the process that maps from logical to physical properties.  Here, there's no corresponding offset property.  We'll have to special case that at the end of EnsurePhysicalProperty.

Is there a bug for re-writing the UA style sheets in terms of logical properties?  We'll need to stick CSS_PROPERTY_ALWAYS_ENABLED_IN_UA_SHEETS on all these properties given they're living behind a pref at the moment.
Assignee: nobody → cam
Status: NEW → ASSIGNED
No, this is 'block-size' and 'inline-size' and their min-* and max-* variants.

'offset-*' correspond to 'top', 'right', 'bottom', and 'left'.
(Assignee)

Comment 3

4 years ago
Oh, indeed.  So these might need a different mechanism to convert to physical properties than I have for the other logical properties we've converted so far, as these are two-property groups rather than four-property groups.
(Assignee)

Comment 5

4 years ago
Jonathan, apart from the margin, padding, border and size properties, are there any others required for the UA style sheet conversion?  And is there a bug for that this one can block?
(Reporter)

Updated

4 years ago
Blocks: 1119475
(Reporter)

Comment 6

4 years ago
I've just filed bug 1119475 on the conversion. Offhand, I'm not aware of other properties required - but we'll find out in the course of going through the stylesheet files. Time to make a start on that...
(Reporter)

Comment 7

4 years ago
Hmm, I suppose we might find that we need logical equivalents of top/left/bottom/right, though the usage I've noticed so far in ua.css looks like we could get away without logicalising it. Ultimately, it seems like these ought to exist, though, even if we don't need them so urgently.
Quick question for (future) documentation: will these properties only be available in UA stylesheets?
Keywords: dev-doc-needed
(Assignee)

Comment 9

4 years ago
(In reply to Jean-Yves Perrier [:teoli] from comment #8)
> Quick question for (future) documentation: will these properties only be
> available in UA stylesheets?

They will be available for content to use (though initially behind a pref).
(Assignee)

Updated

4 years ago
Depends on: 1120284
(Assignee)

Comment 10

4 years ago
Created attachment 8547365 [details] [diff] [review]
Part 1: Implement the {block,inline}-size logical properties.
Attachment #8545169 - Attachment is obsolete: true
Attachment #8547365 - Flags: review?(dbaron)
(Assignee)

Comment 11

4 years ago
Created attachment 8547366 [details] [diff] [review]
Part 2: Implement the max-{block,inline}-size logical properties.
Attachment #8547366 - Flags: review?(dbaron)
(Assignee)

Comment 12

4 years ago
Created attachment 8547368 [details] [diff] [review]
Part 3: Implement the min-{block,inline}-size logical properties.
Attachment #8547368 - Flags: review?(dbaron)
(Assignee)

Comment 13

4 years ago
Created attachment 8547370 [details] [diff] [review]
Part 4: Tests.
Attachment #8547370 - Flags: review?(dbaron)
(Assignee)

Comment 14

4 years ago
Try run for my logical properties branch: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9bf2ce4d8c8c
Comment on attachment 8547365 [details] [diff] [review]
Part 1: Implement the {block,inline}-size logical properties.

I'm not crazy about shipping in this state (i.e., with the hack
in nsRuleNode.cpp here).

We should probably have a bug on (a) adding the keywords to 'height' and
(b) removing the hack in nsRuleNode.cpp.  Would you mind filing that?
It depends both on bug 1113216 and on the resulting followup bug
requested in bug 1113216 comment 9.

I'm inclined to think it should block the bug on shipping vertical
text support, at least for now.  (Maybe not block shipping, but it should
block moving people off of vertical text to other things.)
Attachment #8547365 - Flags: review?(dbaron) → review+
Comment on attachment 8547366 [details] [diff] [review]
Part 2: Implement the max-{block,inline}-size logical properties.

... and that followup bug should mention this nsRuleNode.cpp hack too, and the one in the next patch
Attachment #8547366 - Flags: review?(dbaron) → review+
Comment on attachment 8547366 [details] [diff] [review]
Part 2: Implement the max-{block,inline}-size logical properties.

Oh, and the logical group entry in nsCSSPropList.h should be MaxSize rather than Size.  And your tests should have caught that.
Comment on attachment 8547368 [details] [diff] [review]
Part 3: Implement the min-{block,inline}-size logical properties.

the logical group should be MinSize rather than Size, and you should fix the tests to catch that.
Attachment #8547368 - Flags: review?(dbaron) → review+
Comment on attachment 8547370 [details] [diff] [review]
Part 4: Tests.

>+  gAxisPropertyGroups = [];
>+  ["", "max-", "min-"].forEach(function(aPrefix) {
>+    gAxisPropertyGroups.push({
>+      horizontal: "width", vertical: "height",

I think you need to add aPrefix here.

>+      inline: `${aPrefix}inline-size`, block: `${aPrefix}block-size`,
>+      type: "length",
>+      prerequisites:
>+        make_declaration(gCSSProperties[`${aPrefix}height`].prerequisites)
>+    });
>+  });

(Please test that that makes things fail without the above changes, but pass with them!)


r=dbaron with that
Attachment #8547370 - Flags: review?(dbaron) → review+
(Assignee)

Comment 20

4 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #15)
> I'm not crazy about shipping in this state (i.e., with the hack
> in nsRuleNode.cpp here).

Shipping with vertical text support enabled and with the hack present, or even with vertical text support disabled but the hack present?

> We should probably have a bug on (a) adding the keywords to 'height' and
> (b) removing the hack in nsRuleNode.cpp.  Would you mind filing that?
> It depends both on bug 1113216 and on the resulting followup bug
> requested in bug 1113216 comment 9.

Filed bug 1122253.

> I'm inclined to think it should block the bug on shipping vertical
> text support, at least for now.  (Maybe not block shipping, but it should
> block moving people off of vertical text to other things.)

I couldn't find a bug for shipping vertical text; I made it block bug 45503.
(In reply to Cameron McCormack (:heycam) from comment #20)
> (In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from
> comment #15)
> > I'm not crazy about shipping in this state (i.e., with the hack
> > in nsRuleNode.cpp here).
> 
> Shipping with vertical text support enabled and with the hack present, or
> even with vertical text support disabled but the hack present?

I meant enabling the vertical text pref with the hack present.

> I couldn't find a bug for shipping vertical text; I made it block bug 45503.

Probably bug 1099032.
(Assignee)

Comment 22

4 years ago
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #19)
> (Please test that that makes things fail without the above changes, but pass
> with them!)

Confirmed.
Hi!
In order to assess the documentation need here. Can you confirm/infirm if I understand well this bug? 

It creates: inline-size, block-size, max-inline-size, max-block-size, min-inline-size, and max-block-size.

They are behind a flag, layout.css.vertical-text.enabled (off by default for the moment), but available to any web content.

They aren't part of a Web standard.

Am I correct?
Flags: needinfo?(cam)
I just found this: http://dev.w3.org/csswg/css-logical-props/ My last assertion was incorrect.
(Assignee)

Comment 28

4 years ago
(In reply to Jean-Yves Perrier [:teoli] from comment #26)
> It creates: inline-size, block-size, max-inline-size, max-block-size,
> min-inline-size, and max-block-size.
> 
> They are behind a flag, layout.css.vertical-text.enabled (off by default for
> the moment), but available to any web content.

Yes that's right.
Flags: needinfo?(cam)
(Reporter)

Updated

4 years ago
Depends on: 1131994
See Also: → bug 1132859
You need to log in before you can comment on or make changes to this bug.