Closed Bug 1792608 Opened 2 months ago Closed 1 month ago

text-emphasis-position does not support optional left / right values

Categories

(Firefox :: Untriaged, defect)

Firefox 107
defect

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox108 --- fixed

People

(Reporter: anurag.kalia, Assigned: anurag.kalia)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:105.0) Gecko/20100101 Firefox/105.0

Steps to reproduce:

Logged here: https://wpt.fyi/results/css/css-text-decor/parsing/text-emphasis-position-computed.html?label=master&label=experimental&aligned&view=subtest&q=text%20emphasis%20position

Actual results:

It does not support text-emphasis-position: over or text-emphasis-position: under

Expected results:

It should have worked just like text-emphasis-position: over right / text-emphasis-position: under right, respectively.

OS: Unspecified → All
Hardware: Unspecified → All

I will work on this as per the discussion in the attached links.

Assignee: nobody → anurag.kalia
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Hi emilio! I am back with a new question. I made the changes as you suggested for making left/right in text-emphasis-position optional. I followed the blueprint in your comment and the changes can be seen in the phabricator.

Now, a potential problem I am facing is by reproducing the steps as follows:

  1. In the locally built browser with the changes, navigate to https://developer.mozilla.org/en-US/docs/Web/CSS/text-emphasis-position. You will see the interactive example with text-emphasis-position: over right; selected by default and "happy than right" having blue dots over it.
  2. Open developer console and highlight the element "happy than right" (span#example-element).

Expected: Value of text-emphasis-position for span#example-element should be over right.

Observed: Value of text-emphasis-position for span#example-element is over.

Further Research

I digged into it and figured out that it is happening on using style.cssText = 'text-emphasis-position: over right;' for it and both over right and over are parsed to the same numeric value.

When I checked the docs for style.cssText, I found the following steps (cssom-1/#dom-cssstyledeclaration-csstext):

Setting the cssText attribute must run these steps:

  1. If the computed flag is set, then throw a NoModificationAllowedError exception.
  2. Empty the declarations.
  3. Parse the given value and, if the return value is not the empty list, insert the items in the list into the declarations, in specified order.
  4. Update style attribute for the CSS declaration block.

Basically, we have to "Parse the given value" where over right becomes equivalent to over and when we serialize it back during "Update the style attribute", it turns into over because that it cannot distinguish between the two.

My question is, is it okay? I tried to drill down into the definitions of parsing and could not figure out if this is allowed. Is there any other similar example for optional values for other CSS properties? Moreover, I could not see tests for the same for cssText so that also could not help me here.

Flags: needinfo?(emilio)

Replied to your question in phabricator, but yes, it's ok (and in fact generally the right thing to do) to serialize to the shortest form. See all the #[css(skip_if...)] usage which also behaves the same way.

Flags: needinfo?(emilio)

Hi Emilio! No hurry, but I was worrying if my request on phabricator did not reach you. So, asking for information here too. Thanks! It was about running tests which passed but I am not sure if it was the correct query. So needed confirmation from you for that.

Flags: needinfo?(emilio)

Yeah, sorry, I replied there. Thanks!

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/186bc4e56d14
Make vertical writing mode (left/right) in text-emphasis-position optional r=emilio
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/81300558527a
Annotate a missing passing test.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/6e67171775b3
Simplify other passing test annotations.
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch
You need to log in before you can comment on or make changes to this bug.