David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Description • 10 years ago

I'm filing this as a bug on implementing ruby-position:inter-character, which I think is a lower priority than the rest of ruby-position (bug 1055665) and which is substantially more work.

I think we probably don't need to do this in order to ship CSS Ruby, although I'm willing to entertain arguments to the contrary.

	Tim Guan-tin Chien [:timdream] (please needinfo)
	Comment 1 • 10 years ago

WebKit shipped this feature in Nightly last month.

https://bugs.webkit.org/show_bug.cgi?id=136137

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 2 • 9 years ago

The CSS Ruby spec says that we need to compute writing-mode to vertical-rl for inter-character ruby text container, which means, making writing-mode depend on display and ruby-position.

From the spec perspective, this is fine, because display does not depend on writing-mode in any way. However, in our impl, the current situation is that, writing-mode is in nsStyleVisibility, and display is in nsStyleDisplay, and we have some other properties in nsStyleDisplay which depend on writing-mode (or more specifically, rely on length calculation, which relies on writing-mode to have the right font metrics)

I guess what we probably can do to solve this problem is, moving the properties rely on writing-mode out from nsStyleDisplay, and make nsStyleDisplay be inited before nsStyleVisibility. display itself only depends on float and position. There is no further dependency.

In addition to the dependency problem, writing-mode is an inherited property, while display is not. I suspect that it is awkward to have an inherited property depend on a reset property in our architecture, because it puts us in a dilemma that whether we put the property in an inherited struct or a reset struct won't have an optimal result.

Probably it's fine to have an independent visibility struct for each rtc, though. It doesn't seem to be very large.

BTW, why are we not using bit field for many of the properties which have only limited values? I feel we would save a lot of memory if we do so.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 3 • 9 years ago

Anyway, ruby-position should be moved to nsStyleVisibility. By doing that, we are also enabled to implement bug 1121214.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Comment 4 • 9 years ago

(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #2)
> The CSS Ruby spec says that we need to compute writing-mode to vertical-rl
> for inter-character ruby text container, which means, making writing-mode
> depend on display and ruby-position.
> 
> From the spec perspective, this is fine, because display does not depend on
> writing-mode in any way. However, in our impl, the current situation is
> that, writing-mode is in nsStyleVisibility, and display is in
> nsStyleDisplay, and we have some other properties in nsStyleDisplay which
> depend on writing-mode (or more specifically, rely on length calculation,
> which relies on writing-mode to have the right font metrics)
> 
> I guess what we probably can do to solve this problem is, moving the
> properties rely on writing-mode out from nsStyleDisplay, and make
> nsStyleDisplay be inited before nsStyleVisibility. display itself only
> depends on float and position. There is no further dependency.

We probably need to either split nsStyleDisplay, or move some properties from nsStyleDisplay to a different reset struct.

Before we do either, I'd like somebody to finish and land bug 1122781.


> In addition to the dependency problem, writing-mode is an inherited
> property, while display is not. I suspect that it is awkward to have an
> inherited property depend on a reset property in our architecture, because
> it puts us in a dilemma that whether we put the property in an inherited
> struct or a reset struct won't have an optimal result.

I don't see why that would be an issue; the structs match what the properties do by default.

> Probably it's fine to have an independent visibility struct for each rtc,
> though. It doesn't seem to be very large.
> 
> BTW, why are we not using bit field for many of the properties which have
> only limited values? I feel we would save a lot of memory if we do so.

I don't think it would be much, especially once you consider rounding of allocation sizes.  Also, there's value in having all of the property values have addresses.  (We used to have a small number of them be bitfields, but we stopped.)

The enumerated properties are also the smallest ones; bigger memory savings could be found elsewhere.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 5 • 9 years ago

It seems that we are still unable to implement this, because rtc could be (and usually be) an anonymous box, and ruby could be an anonymous either. Because of this fact, it is impossible to fix the writing-mode before knowing the display value for the same element, since we know nothing about its potential container untill we know the display value.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 6 • 9 years ago

I guess we probably have to handle the cyclic dependencies between display and writing-mode explicitly, even if it looks broken.

I intend to break nsStyleDisplay into two parts, one part keeps nsStyleDisplay, and the other part can probably be called nsStyleVisual or nsStyleRendering, which includes the properties depend on writing-mode, including rect, transition-related, animation-relateds, transform-relateds.

dbaron, if you have no objection on this plan, I'd like to start working on it this way.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Comment 7 • 9 years ago

What is the behavior that you're planning to implement by doing this?

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 8 • 9 years ago

To make writing-mode be computed to vertical-rl when display is computed to ruby-text or ruby-text-container.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Comment 9 • 9 years ago

So which properties in nsStyleDisplay depend on writing-mode?

It might be preferable to move just those properties to nsStylePosition rather than creating an entirely new struct.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 10 • 9 years ago

I thought transition and animation do but it seems I was wrong.

After checking the code in nsRuleNode again, I think the properties depend on writing-mode are:
* clip (for the rect length computation)
* transform-origin (coord)
* perspective-origin (coord)
* perspective (coord)

I guess we should also move: transform, backface-visibility, and transform-style, as they are usually used together with the properties mentioned above.

Agree with this movement?

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Comment 11 • 9 years ago

Yeah, moving that set to nsStylePosition sounds fine.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 12 • 9 years ago

nsStyleDisplay::HasTransformStyle checks mWillChangeBitField and transform properties. Should we also move fields related to will-change to nsStylePosition as well, so that we can keep that method as-is?

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla) Reporter
	Comment 13 • 9 years ago

I guess so.

	Jonathan Watt [:jwatt]
	Comment 14 • 6 years ago

Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)