Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stylo: Properly support -moz-script-size-multiplier, -moz-script-level, and -moz-script-min-size #16539

Merged
merged 2 commits into from
Apr 20, 2017

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Apr 20, 2017

r=heycam https://bugzilla.mozilla.org/show_bug.cgi?id=1355427

(copying over the relevant commit message so that it doesn't get lost in the vcssync)

scriptlevel is a property that affects how font-size is inherited. If scriptlevel is
+1, for example, it will inherit as the script size multiplier times
the parent font. This does not affect cases where the font-size is
explicitly set.

However, this transformation is not allowed to reduce the size below
scriptminsize. If this inheritance will reduce it to below
scriptminsize, it will be set to scriptminsize or the parent size,
whichever is smaller (the parent size could be smaller than the min size
because it was explicitly specified).

Now, within a node that has inherited a font-size which was
crossing scriptminsize once the scriptlevel was applied, a negative
scriptlevel may be used to increase the size again.

This should work, however if we have already been capped by the
scriptminsize multiple times, this can lead to a jump in the size.

For example, if we have text of the form:

huge large medium small tiny reallytiny tiny small medium huge

which is represented by progressive nesting and scriptlevel values of
+1 till the center after which the scriptlevel is -1, the "tiny"s should
be the same size, as should be the "small"s and "medium"s, etc.

However, if scriptminsize kicked it at around "medium", then
medium/tiny/reallytiny will all be the same size (the min size).
A -1 scriptlevel change after this will increase the min size by the
multiplier, making the second tiny larger than medium.

Instead, we wish for the second "tiny" to still be capped by the script
level, and when we reach the second "large", it should be the same size
as the original one.

We do this by cascading two separate font sizes. The font size (mSize)
is the actual displayed font size. The unconstrained font size
(mScriptUnconstrainedSize) is the font size in the situation where
scriptminsize never applied.

We calculate the proposed inherited font size based on scriptlevel and
the parent unconstrained size, instead of using the parent font size.
This is stored in the node's unconstrained size and will also be stored
in the font size provided that it is above the min size.

All of this only applies when inheriting. When the font size is
manually set, scriptminsize does not apply, and both the real and
unconstrained size are set to the explicit value. However, if the font
size is manually set to an em or percent unit, the unconstrained size
will be set to the value of that unit computed against the parent
unconstrained size, whereas the font size will be set computing against
the parent font size.


This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/gecko.mako.rs, components/style/values/computed/length.rs, components/style/properties/helpers.mako.rs, components/style/properties/longhand/font.mako.rs, components/style/values/specified/length.rs and 1 more
  • @emilio: components/style/properties/gecko.mako.rs, components/style/values/computed/length.rs, components/style/properties/helpers.mako.rs, components/style/properties/longhand/font.mako.rs, components/style/values/specified/length.rs and 1 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 20, 2017
@highfive
Copy link

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

@Manishearth
Copy link
Member Author

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

📌 Commit 75dbe9c has been approved by heycam

@highfive highfive assigned heycam and unassigned mbrubeck Apr 20, 2017
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 20, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 75dbe9c with merge 732682b...

bors-servo pushed a commit that referenced this pull request Apr 20, 2017
stylo: Properly support -moz-script-size-multiplier, -moz-script-level, and -moz-script-min-size

r=heycam https://bugzilla.mozilla.org/show_bug.cgi?id=1355427

(copying over the relevant commit message so that it doesn't get lost in the vcssync)

scriptlevel is a property that affects how font-size is inherited. If scriptlevel is
+1, for example, it will inherit as the script size multiplier times
the parent font. This does not affect cases where the font-size is
explicitly set.

However, this transformation is not allowed to reduce the size below
scriptminsize. If this inheritance will reduce it to below
scriptminsize, it will be set to scriptminsize or the parent size,
whichever is smaller (the parent size could be smaller than the min size
because it was explicitly specified).

Now, within a node that has inherited a font-size which was
crossing scriptminsize once the scriptlevel was applied, a negative
scriptlevel may be used to increase the size again.

This should work, however if we have already been capped by the
scriptminsize multiple times, this can lead to a jump in the size.

For example, if we have text of the form:

huge large medium small tiny reallytiny tiny small medium huge

which is represented by progressive nesting and scriptlevel values of
+1 till the center after which the scriptlevel is -1, the "tiny"s should
be the same size, as should be the "small"s and "medium"s, etc.

However, if scriptminsize kicked it at around "medium", then
medium/tiny/reallytiny will all be the same size (the min size).
A -1 scriptlevel change after this will increase the min size by the
multiplier, making the second tiny larger than medium.

Instead, we wish for the second "tiny" to still be capped by the script
level, and when we reach the second "large", it should be the same size
as the original one.

We do this by cascading two separate font sizes. The font size (mSize)
is the actual displayed font size. The unconstrained font size
(mScriptUnconstrainedSize) is the font size in the situation where
scriptminsize never applied.

We calculate the proposed inherited font size based on scriptlevel and
the parent unconstrained size, instead of using the parent font size.
This is stored in the node's unconstrained size and will also be stored
in the font size provided that it is above the min size.

All of this only applies when inheriting. When the font size is
manually set, scriptminsize does not apply, and both the real and
unconstrained size are set to the explicit value. However, if the font
size is manually set to an em or percent unit, the unconstrained size
will be set to the value of that unit computed against the parent
unconstrained size, whereas the font size will be set computing against
the parent font size.
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 20, 2017
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 20, 2017
scriptlevel is a property that affects how font-size is inherited. If scriptlevel is
+1, for example, it will inherit as the script size multiplier times
the parent font. This does not affect cases where the font-size is
explicitly set.

However, this transformation is not allowed to reduce the size below
scriptminsize. If this inheritance will reduce it to below
scriptminsize, it will be set to scriptminsize or the parent size,
whichever is smaller (the parent size could be smaller than the min size
because it was explicitly specified).

Now, within a node that has inherited a font-size which was
crossing scriptminsize once the scriptlevel was applied, a negative
scriptlevel may be used to increase the size again.

This should work, however if we have already been capped by the
scriptminsize multiple times, this can lead to a jump in the size.

For example, if we have text of the form:

huge large medium small tiny reallytiny tiny small medium huge

which is represented by progressive nesting and scriptlevel values of
+1 till the center after which the scriptlevel is -1, the "tiny"s should
be the same size, as should be the "small"s and "medium"s, etc.

However, if scriptminsize kicked it at around "medium", then
medium/tiny/reallytiny will all be the same size (the min size).
A -1 scriptlevel change after this will increase the min size by the
multiplier, making the second tiny larger than medium.

Instead, we wish for the second "tiny" to still be capped by the script
level, and when we reach the second "large", it should be the same size
as the original one.

We do this by cascading two separate font sizes. The font size (mSize)
is the actual displayed font size. The unconstrained font size
(mScriptUnconstrainedSize) is the font size in the situation where
scriptminsize never applied.

We calculate the proposed inherited font size based on scriptlevel and
the parent unconstrained size, instead of using the parent font size.
This is stored in the node's unconstrained size and will also be stored
in the font size provided that it is above the min size.

All of this only applies when inheriting. When the font size is
manually set, scriptminsize does not apply, and both the real and
unconstrained size are set to the explicit value. However, if the font
size is manually set to an em or percent unit, the unconstrained size
will be set to the value of that unit computed against the parent
unconstrained size, whereas the font size will be set computing against
the parent font size.

MozReview-Commit-ID: 820BIWqno3L
@Manishearth
Copy link
Member Author

@bors-servo r=heycam

@bors-servo
Copy link
Contributor

📌 Commit 3162bab has been approved by heycam

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 20, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 3162bab with merge 7919e59...

bors-servo pushed a commit that referenced this pull request Apr 20, 2017
stylo: Properly support -moz-script-size-multiplier, -moz-script-level, and -moz-script-min-size

r=heycam https://bugzilla.mozilla.org/show_bug.cgi?id=1355427

(copying over the relevant commit message so that it doesn't get lost in the vcssync)

scriptlevel is a property that affects how font-size is inherited. If scriptlevel is
+1, for example, it will inherit as the script size multiplier times
the parent font. This does not affect cases where the font-size is
explicitly set.

However, this transformation is not allowed to reduce the size below
scriptminsize. If this inheritance will reduce it to below
scriptminsize, it will be set to scriptminsize or the parent size,
whichever is smaller (the parent size could be smaller than the min size
because it was explicitly specified).

Now, within a node that has inherited a font-size which was
crossing scriptminsize once the scriptlevel was applied, a negative
scriptlevel may be used to increase the size again.

This should work, however if we have already been capped by the
scriptminsize multiple times, this can lead to a jump in the size.

For example, if we have text of the form:

huge large medium small tiny reallytiny tiny small medium huge

which is represented by progressive nesting and scriptlevel values of
+1 till the center after which the scriptlevel is -1, the "tiny"s should
be the same size, as should be the "small"s and "medium"s, etc.

However, if scriptminsize kicked it at around "medium", then
medium/tiny/reallytiny will all be the same size (the min size).
A -1 scriptlevel change after this will increase the min size by the
multiplier, making the second tiny larger than medium.

Instead, we wish for the second "tiny" to still be capped by the script
level, and when we reach the second "large", it should be the same size
as the original one.

We do this by cascading two separate font sizes. The font size (mSize)
is the actual displayed font size. The unconstrained font size
(mScriptUnconstrainedSize) is the font size in the situation where
scriptminsize never applied.

We calculate the proposed inherited font size based on scriptlevel and
the parent unconstrained size, instead of using the parent font size.
This is stored in the node's unconstrained size and will also be stored
in the font size provided that it is above the min size.

All of this only applies when inheriting. When the font size is
manually set, scriptminsize does not apply, and both the real and
unconstrained size are set to the explicit value. However, if the font
size is manually set to an em or percent unit, the unconstrained size
will be set to the value of that unit computed against the parent
unconstrained size, whereas the font size will be set computing against
the parent font size.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16539)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: heycam
Pushing 7919e59 to master...

@bors-servo bors-servo merged commit 3162bab into servo:master Apr 20, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 20, 2017
@Manishearth Manishearth deleted the stylo-scriptminsize branch April 20, 2017 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants