Closed Bug 1800952 Opened 2 years ago Closed 2 years ago

Computed 'column-rule' serialization has wrong width

Categories

(Core :: CSS Parsing and Computation, defect, P3)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
110 Branch
Tracking Flags:
Tracking Status
firefox110 --- fixed

People

(Reporter: Oriol, Assigned: Oriol)

References

Details

Attachments

(1 file)

Bug 1800952 - Compute column-rule-width to 0 when column-rule-style is none. r?emilio
48 bytes, text/x-phabricator-request
Details | Review 
document.body.style.columnRule = "10px";
getComputedStyle(document.body).columnRuleWidth; // "0px" 
getComputedStyle(document.body).columnRule; // "10px none rgb(0, 0, 0)"

Like, sure, both 0px and 10px widths compute to 0px due to the style being none.
But it looks very wrong that the longhand says 0px and the shorthand says 10px.
A similar case is border, which has a sane behavior:

document.body.style.border = "10px";
getComputedStyle(document.body).borderWidth; // "0px" 
getComputedStyle(document.body).border; // "0px none rgb(0, 0, 0)"

Another testcase:

<!DOCTYPE html>
<div style="width: max-content; columns: 2; column-rule: 10px none red">
  <div>a</div>
  <div style="columns: 2; column-rule: inherit; column-rule-style: solid">
    <div>b</div>
    <div>c</div>
  </div>
</div>

Expected: no red.
Actual: red line between "b" and "c".

From https://drafts.csswg.org/css-multicol/#crs

The none value forces the computed value of column-rule-width to be 0.

So column-rule: inherit should inherit column-rule-width: 0px, but it inherits the specified 10px.

Yes, this is because we implement that "used column rule width" bit of the spec wrong indeed. https://searchfox.org/mozilla-central/rev/2f47e3dacf0d773e9c7f363cecf10cfbea490679/layout/style/nsStyleStruct.h#2009-2011 is just wrong, that's the "used" column-rule width.

This should be easy to fix by keeping both the "pre" and "post" computed values like we do for borders: https://searchfox.org/mozilla-central/rev/2f47e3dacf0d773e9c7f363cecf10cfbea490679/servo/components/style/properties/gecko.mako.rs#645

Severity: -- → S3
Priority: -- → P3

Thanks for the pointers, it's also analogous to outline-width.
Doesn't seem much urgent, so I'll take the bug and send a patch in December during a codecamp.

Assignee: nobody → oriol-bugzilla
Status: NEW → ASSIGNED
Pushed by oriol-bugzilla@hotmail.com: https://hg.mozilla.org/integration/autoland/rev/5c04131a0427 Compute column-rule-width to 0 when column-rule-style is none. r=emilio

When my patch was rebased to tip, somehow a bunch of unrelated changes appeared in ServoCSSPropList.mako.py
I will rebase manually and try again.

Flags: needinfo?(oriol-bugzilla)
Pushed by oriol-bugzilla@hotmail.com: https://hg.mozilla.org/integration/autoland/rev/370f2bb05766 Compute column-rule-width to 0 when column-rule-style is none. r=emilio

Backed out for causing mass build bustages on ServoCSSPropList.mako.py.

Push with failures

Failure log

Backout link

[task 2022-12-14T22:40:20.174Z] 22:40:20     INFO -  gmake[3]: Entering directory '/builds/worker/workspace/obj-build'
[task 2022-12-14T22:40:20.174Z] 22:40:20     INFO -  ./ServoCSSPropList.py.stub
[task 2022-12-14T22:40:20.175Z] 22:40:20     INFO -  /builds/worker/workspace/obj-build/_virtualenvs/build/bin/python -m mozbuild.action.file_generate /builds/worker/checkouts/gecko/layout/style/GenerateServoCSSPropList.py generate_data layout/style/ServoCSSPropList.py layout/style/.deps/ServoCSSPropList.py.pp layout/style/.deps/ServoCSSPropList.py.stub /builds/worker/checkouts/gecko/layout/style/ServoCSSPropList.mako.py
[task 2022-12-14T22:40:20.175Z] 22:40:20    ERROR -  Traceback (most recent call last):
[task 2022-12-14T22:40:20.176Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/build.py", line 148, in render
[task 2022-12-14T22:40:20.176Z] 22:40:20     INFO -      template = Template(
[task 2022-12-14T22:40:20.176Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/template.py", line 331, in __init__
[task 2022-12-14T22:40:20.176Z] 22:40:20     INFO -      (code, module) = _compile_text(self, text, filename)
[task 2022-12-14T22:40:20.177Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/template.py", line 733, in _compile_text
[task 2022-12-14T22:40:20.177Z] 22:40:20     INFO -      source, lexer = _compile(
[task 2022-12-14T22:40:20.177Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/template.py", line 712, in _compile
[task 2022-12-14T22:40:20.177Z] 22:40:20     INFO -      node = lexer.parse()
[task 2022-12-14T22:40:20.178Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/lexer.py", line 272, in parse
[task 2022-12-14T22:40:20.178Z] 22:40:20     INFO -      if self.match_python_block():
[task 2022-12-14T22:40:20.178Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/lexer.py", line 408, in match_python_block
[task 2022-12-14T22:40:20.178Z] 22:40:20     INFO -      text, end = self.parse_until_text(False, r"%>")
[task 2022-12-14T22:40:20.178Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/servo/components/style/properties/Mako-1.1.2-py2.py3-none-any.whl/mako/lexer.py", line 143, in parse_until_text
[task 2022-12-14T22:40:20.179Z] 22:40:20     INFO -      raise exceptions.SyntaxException(
[task 2022-12-14T22:40:20.179Z] 22:40:20     INFO -  SyntaxException: Expected: %> in file '/builds/worker/checkouts/gecko/layout/style/ServoCSSPropList.mako.py' at line: 171 char: 66
[task 2022-12-14T22:40:20.179Z] 22:40:20    ERROR -  Traceback (most recent call last):
[task 2022-12-14T22:40:20.179Z] 22:40:20     INFO -    File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main
[task 2022-12-14T22:40:20.179Z] 22:40:20     INFO -      return _run_code(code, main_globals, None,
[task 2022-12-14T22:40:20.180Z] 22:40:20     INFO -    File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
[task 2022-12-14T22:40:20.180Z] 22:40:20     INFO -      exec(code, run_globals)
[task 2022-12-14T22:40:20.180Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/file_generate.py", line 156, in <module>
[task 2022-12-14T22:40:20.180Z] 22:40:20     INFO -      sys.exit(log_build_task(main, sys.argv[1:]))
[task 2022-12-14T22:40:20.180Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/util.py", line 18, in log_build_task
[task 2022-12-14T22:40:20.181Z] 22:40:20     INFO -      return f(*args, **kwargs)
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/python/mozbuild/mozbuild/action/file_generate.py", line 100, in main
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -      ret = module.__dict__[method](
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -    File "/builds/worker/checkouts/gecko/layout/style/GenerateServoCSSPropList.py", line 20, in generate_data
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -      subprocess.check_output(
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -    File "/usr/lib/python3.9/subprocess.py", line 424, in check_output
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -      return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -    File "/usr/lib/python3.9/subprocess.py", line 528, in run
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -      raise CalledProcessError(retcode, process.args,
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -  subprocess.CalledProcessError: Command '['/builds/worker/workspace/obj-build/_virtualenvs/build/bin/python', '/builds/worker/checkouts/gecko/servo/components/style/properties/build.py', 'gecko', 'geckolib', '/builds/worker/checkouts/gecko/layout/style/ServoCSSPropList.mako.py']' returned non-zero exit status 1.
[task 2022-12-14T22:40:20.183Z] 22:40:20    ERROR -  gmake[3]: *** [backend.mk:526: layout/style/.deps/ServoCSSPropList.py.stub] Error 1
[task 2022-12-14T22:40:20.183Z] 22:40:20     INFO -  gmake[3]: Leaving directory '/builds/worker/workspace/obj-build'
Flags: needinfo?(oriol-bugzilla)

Again the same problem, my patch is altered when I try to land it.
https://phabricator.services.mozilla.com/D164549?vs=659554&id=659582

Flags: needinfo?(oriol-bugzilla)

(In reply to Oriol Brufau [:Oriol] from comment #11)

Again the same problem, my patch is altered when I try to land it.
https://phabricator.services.mozilla.com/D164549?vs=659554&id=659582

Hey, do you know what can we do to bypass this? Seems some sort of auto-formatting going on? That file isn't a regular python file, it uses a templating engine and the formatter is making it not build.

Flags: needinfo?(glob)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #12)

Hey, do you know what can we do to bypass this? Seems some sort of auto-formatting going on? That file isn't a regular python file, it uses a templating engine and the formatter is making it not build.

Lando applies autoformatting on landing by running mach lint --fix (likely to be changed soon to mach format --fix).

Ideally the template files should be renamed so their file extension matches their content (ie. .mako.py → .py.mako, .mako.rs → .rs.mako) but I suspect that's an unsatisfactory amount of work.

Adding an exclusion to https://searchfox.org/mozilla-central/source/tools/lint/black.yml is probably the simplest fix here. Note that all of servo is excluded in rustfmt's config (which is why this issue doesn't happen for the .mako.rs files).

Flags: needinfo?(glob)

I've proposed extra exclude rules for rust and python formater in https://bugzilla.mozilla.org/show_bug.cgi?id=1805839

Thanks Serge! Though per the patch in bug 1805839, that file (ServoCSSPropList.mako.py) was already excluded. Is Lando somehow not respecting that properly? Worth trying to land this after Serge's fix lands tho.

Flags: needinfo?(glob)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)

Thanks Serge! Though per the patch in bug 1805839, that file (ServoCSSPropList.mako.py) was already excluded.

So it is! I have no idea how I missed that!

Is Lando somehow not respecting that properly? Worth trying to land this after Serge's fix lands tho.

Literally all Lando does is run mach lint --fix --outgoing: https://github.com/mozilla-conduit/lando-api/blob/60d3795e798394d1be5bbedeb3a0c7d0f85f0810/landoapi/hg.py#L399

If exclusions aren't being handled correctly then it's an issue with mach, not Lando.

Flags: needinfo?(glob)

https://bugzilla.mozilla.org/show_bug.cgi?id=1805839 updated, it wasn't black's fault, but flake8's .

Depends on: 1805839
Pushed by oriol-bugzilla@hotmail.com: https://hg.mozilla.org/integration/autoland/rev/b13639699fa7 Compute column-rule-width to 0 when column-rule-style is none. r=emilio

https://hg.mozilla.org/mozilla-central/rev/b13639699fa7

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
status-firefox110: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: