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

Backport several style changes from Gecko (5) #30099

Merged
merged 87 commits into from
Aug 16, 2023
Merged

Conversation

Loirooriol
Copy link
Contributor

@Loirooriol Loirooriol commented Aug 15, 2023

This continues #29848.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

emilio and others added 30 commits August 15, 2023 06:13
These bits are write-only, actually, and we don't even need to read
them.

Differential Revision: https://phabricator.services.mozilla.com/D141888
This is a stub that only matches "standard" for all platforms.

Differential Revision: https://phabricator.services.mozilla.com/D141053
They are just convenience for :root[lwthemetextcolor="light"] (and dark,
respectively), but they generally shouldn't be used for dark mode
theming. In the past it was the only way to do it but now we have
prefers-color-scheme.

While at it, change lwthemetextcolor to be "lwtheme-brighttext" for
consistency with similar code we have for popups etc, and move it to
_setDarkModeAttributes.

While at it, remove layout.css.moz-lwtheme.content.enabled (which is
false always, we unshipped these from content successfully).

Differential Revision: https://phabricator.services.mozilla.com/D141593
We successfully removed these from content in bug 1740230 (Firefox 96).

Differential Revision: https://phabricator.services.mozilla.com/D141727
…m controls

I forgot we were doing this "revert-or-initial" shenanigans (which is needed
for stuff like link colors to be honored), so we need to early-return.

Use a more explicit test rather than a reftest for this.

Differential Revision: https://phabricator.services.mozilla.com/D142063
We keep getting this pattern of properties that have a set of joint and
disjoint flags, and copy-pasting or writing the same parsing and
serialization code in slightly different ways.

container-type is one such type, and I think we should have a single way
of dealing with this, thus implement deriving for various traits for
bitflags, with an attribute that says which flags are single vs mixed.

See docs and properties I ported. The remaining ones I left TODOs with,
they are a bit trickier but can be ported with some care.

Differential Revision: https://phabricator.services.mozilla.com/D142418
…CSS properties

Two noteworthy details that may seem random otherwise:

 * Moving values around in nsStyleDisplay is needed so that the struct
   remains under the size limit that we have to avoid jumping allocator
   buckets.

 * All the test expectation churn is because tests depend on
   `container-type: size` parsing to run, and now they run. Tests for
   the relevant bits I implemented are passing, with the only exception
   of some `container-name-computed.html` failures which are
   w3c/csswg-drafts#7181. Safari agrees with
   us there.

Other notes when looking at the spec and seeing how it matches the
implementation:

 * `container` syntax doesn't match spec, but matches tests and sanity:
   w3c/csswg-drafts#7180

 * `container-type` syntax doesn't _quite_ match spec, but matches tests
   and I think it's a spec bug since the definition for the missing
   keyword is gone:
   w3c/csswg-drafts#7179

Differential Revision: https://phabricator.services.mozilla.com/D142419
…f presentation hints

MANUAL PUSH: Orange fix CLOSED TREE
Remember whether we have already de-duplicated them once and avoid doing
that again.

This is an alternative approach that doesn't add overhead to attribute
setting in the general case.

Differential Revision: https://phabricator.services.mozilla.com/D142813
This allows popups and sidebars to use the chrome preferred
color-scheme.

This moves the responsibility of setting the content-preferred color
scheme to the appropriate browsers to the front-end (via tabs.css).

We still return the PreferredColorSchemeForContent() when there's no
pres context (e.g., for display:none in-process iframes). We could
potentially move a bunch of the pres-context data to the document
instead, but that should be acceptable IMO as for general web content
there's no behavior change in any case.

Differential Revision: https://phabricator.services.mozilla.com/D142578
For now parse a MediaFeatureCondition. That needs being made more
specific, but that is probably worth its own patch.

Differential Revision: https://phabricator.services.mozilla.com/D143192
This makes the worst case for cascade performance slightly more
expensive (4 rather than three declaration walks), but my hope is that
it will make the average case faster, since the best case is now just
two walks instead of three, and writing mode properties are somewhat
rare.

This needs a test, but needs to wait until the writing-mode dependent
viewport units land (will wait to land with a test).

Differential Revision: https://phabricator.services.mozilla.com/D143261
I was unable to change the BLOOM_KEY field to no longer be leaked, as the TLS
is also accessed on the main thread, which is not exited before the leak
checker shuts down.

Differential Revision: https://phabricator.services.mozilla.com/D143529
Before bug 1763750, we unconditionally called compute_writing_mode,
which got the writing mode from the cascade mode for visited styles.

However after that bug we only do that if we apply any
writing-mode-related property.

We could just call compute_writing_mode unconditionally, but instead it
seems better to skip all that work for visited cascade and reuse the
mechanism introduced in that bug to only apply the visited-dependent
longhands.

We assert that all visited-dependent longhands are "late" longhands, so
as to also avoid applying the font group and such.

Differential Revision: https://phabricator.services.mozilla.com/D143490
I should've caught this when reviewing the new viewport units but alas :-)

Differential Revision: https://phabricator.services.mozilla.com/D143856
This patch:

  * Removes generic <ident> support for media features. These were used
    for some privileged media features but are no longer used.

  * Simplifies media feature getters by shifting the responsibility of
    dealing with RangeOrOperator to the caller. This makes it easier to
    implement container-query / mediaqueries-4 syntax, and also cleans up
    the code a bunch.

There should be no change in behavior.

Differential Revision: https://phabricator.services.mozilla.com/D144051
This mostly just moves code around, to minimize potential behavior
changes. There are some cleanups that we should try to do long term
(this "have an array with n different counts" is pretty weird).

But for now this should unblock people.

The destination struct (nsStyleUIReset) was chosen mainly because it's
small and non-inherited, and it doesn't seem like a worse place than
nsStyleDisplay.

Differential Revision: https://phabricator.services.mozilla.com/D144183
emilio and others added 12 commits August 15, 2023 21:55
This doesn't change behavior because we only use them for images that
have no clamping.

Depends on D147008

Differential Revision: https://phabricator.services.mozilla.com/D147511
It unconditionally matches on all platforms, so it's not returning any useful information.

Depends on D147689

Differential Revision: https://phabricator.services.mozilla.com/D147690
…her than ComplexColorRatios

This among other things preserves the right color-space when
interpolating currentColor.

Differential Revision: https://phabricator.services.mozilla.com/D147512
Recompute the input region when resizing the widget and so on, and use
it to check for rollups.

Depends on D148211

Differential Revision: https://phabricator.services.mozilla.com/D148222
@Loirooriol
Copy link
Contributor Author

These are the additions that I had to add to compile: a1c8341, a9afa00, 01e1801, 312ace6, e2dc8c8, 81a5f34, 913edb3, 6faef7f, a1bb844, 90bc74d, 8325a56, 3edc295, 42d82e8, 2d40b8f, 9384831, 0142678, b9e40d7, e3e5123, 4cfcfff, 2c74184.

For 42d82e8 I don't have much idea if this unsafe code is actually safe or makes sense:

    fn set_selector_flags(&self, flags: ElementSelectorFlags) {
        #[allow(unsafe_code)]
        unsafe {
            Dom::from_ref(self.deref())
                .to_layout()
                .insert_selector_flags(flags);
        }
    }

but it can always be addressed in a follow-up if it's problematic.

@Loirooriol Loirooriol marked this pull request as ready for review August 15, 2023 20:30
@Loirooriol Loirooriol added this pull request to the merge queue Aug 15, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 15, 2023
@Loirooriol Loirooriol added this pull request to the merge queue Aug 15, 2023
Merged via the queue into servo:master with commit 8e15389 Aug 16, 2023
10 checks passed
@Loirooriol Loirooriol deleted the sync branch August 16, 2023 00:22
@Loirooriol Loirooriol restored the sync branch August 16, 2023 05:02
@Loirooriol
Copy link
Contributor Author

Ah, the commits got merged together, which will be bad for bisects. I will revert and reland as a rebase

Loirooriol added a commit that referenced this pull request Aug 16, 2023
Loirooriol added a commit to Loirooriol/servo that referenced this pull request Aug 16, 2023
This reverts commit 8e15389.

Should have landed the commits separately instead of merging them
together into a single one.
github-merge-queue bot pushed a commit that referenced this pull request Aug 16, 2023
@Loirooriol Loirooriol deleted the sync branch August 16, 2023 20: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.

10 participants