Open Bug 1055672 (inter-character-ruby) Opened 6 years ago Updated Last year

implement ruby-position: inter-character

Categories

(Core :: Layout: Ruby, defect)

defect
Not set

Tracking

()

People

(Reporter: dbaron, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: dev-doc-needed, parity-safari)

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.
Alias: inter-character-ruby
WebKit shipped this feature in Nightly last month.

https://bugs.webkit.org/show_bug.cgi?id=136137
Whiteboard: [parity-safari]
Depends on: 1055665
Depends on: 1115264
Depends on: 1121195
Depends on: 1121214
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.
Flags: needinfo?(dbaron)
Anyway, ruby-position should be moved to nsStyleVisibility. By doing that, we are also enabled to implement bug 1121214.
(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.
Flags: needinfo?(dbaron)
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.
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.
Flags: needinfo?(dbaron)
What is the behavior that you're planning to implement by doing this?
Flags: needinfo?(dbaron)
To make writing-mode be computed to vertical-rl when display is computed to ruby-text or ruby-text-container.
Flags: needinfo?(dbaron)
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.
Flags: needinfo?(dbaron)
Flags: needinfo?(quanxunzhen)
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?
Flags: needinfo?(dbaron)
Flags: needinfo?(quanxunzhen)
Yeah, moving that set to nsStylePosition sounds fine.
Flags: needinfo?(dbaron)
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?
Flags: needinfo?(dbaron)
I guess so.
Flags: needinfo?(dbaron)
Depends on: 1139283
Depends on: 1395766
Depends on: 1395774
See Also: → 1395777
Mass bug change to replace various 'parity' whiteboard flags with the new canonical keywords. (See bug 1443764 comment 13.)
Keywords: parity-safari
Whiteboard: [parity-safari]
Component: Layout: Block and Inline → Layout: Ruby
You need to log in before you can comment on or make changes to this bug.