Closed Bug 2004688 Opened 2 months ago Closed 2 months ago

Refactor Has-variant CSS serialization to use existing serialize_selector_list helper

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
148 Branch
Tracking Status
firefox148 --- fixed

People

(Reporter: nabeyang, Assigned: nabeyang)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/143.0.0.0 Safari/537.36

Steps to reproduce:

In parser.rs, the Has(ref list) arm was manually iterating over the list of RelativeSelectors to call selector.to_css(dest) with comma+space separation.
I replaced this manual loop with a call to the existing helper function serialize_selector_list, passing list.iter().map(|rel| &rel.selector). This unifies the serialization logic for selector lists and reduces duplicated code.

Actual results:

Currently, there are essentially two different implementations for serializing selector-lists with identical semantics: one for Is/Where/Negation variants (via serialize_selector_list), and one custom loop for Has.
This duplication increases maintenance burden and risks subtle inconsistency. For example, if formatting rules (spacing, comma separation) are ever adjusted in serialize_selector_list, the Has branch might not follow the update, leading to inconsistent CSS output.

Expected results:

Selector-list serialization should be implemented uniformly across all relevant variants; in particular, the Has variant should reuse the existing serialize_selector_list helper just like other variants. This ensures consistency, reduces duplicated code, and simplifies future maintenance.
The change should not alter any functional behavior (CSS output stays identical), but makes the code easier to maintain and less error-prone.

Replace manual loop in the Has(ref list) arm with a call to the existing
serialize_selector_list helper. This removes duplication and ensures
consistent selector-list serialization across variants (Is/Where/Negation/Has),
without changing any functional output.

No behavioral change expected; CSS output remains identical. The refactoring
improves maintainability and reduces chances of formatting divergence in
future.

Assignee: nobody → nabeyang
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Pushed by ealvarez@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/a06fb5aec4e9 https://hg.mozilla.org/integration/autoland/rev/6aee44e658bf – Refactor Has-variant CSS serialization to use serialize_selector_list r=firefox-style-system-reviewers,dshin
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 148 Branch

The documentation comments for combinator_at_match_order and
combinator_at_parse_order described index directions opposite to their
actual implementations.

This change updates the Rustdoc comments so that the documented index
semantics (zero-indexed from the left or right) match the behavior of
each function.

Comment on attachment 9532855 [details]
Bug 2004688 - Fix swapped index direction comments for combinator accessors r=dshin

Revision D276342 was moved to bug 2005876. Setting attachment 9532855 [details] to obsolete.

Attachment #9532855 - Attachment is obsolete: true
QA Whiteboard: [qa-triage-done-c149/b148]
You need to log in before you can comment on or make changes to this bug.