Bug 254585 - overriding style has no effect until original style is deactivated/unchecked in web inspector
Summary: overriding style has no effect until original style is deactivated/unchecked ...
Status: RESOLVED WORKSFORME
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 16
Hardware: All All
: P2 Major
Assignee: Razvan Caliman
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-28 07:15 PDT by Matt Sephton
Modified: 2023-07-07 07:06 PDT (History)
5 users (show)

See Also:


Attachments
screen recording of issue (13.52 MB, video/quicktime)
2023-03-28 07:15 PDT, Matt Sephton
no flags Details
test case (1.63 KB, text/html)
2023-03-29 01:31 PDT, Razvan Caliman
no flags Details
test case screen recording (4.37 MB, video/quicktime)
2023-03-29 01:45 PDT, Razvan Caliman
no flags Details
how i apply user stylesheet (84.60 KB, image/png)
2023-03-29 07:34 PDT, Matt Sephton
no flags Details
my user stylesheet to workaround the issue (124 bytes, text/css)
2023-03-29 07:42 PDT, Matt Sephton
no flags Details
Screen recording of Web Inspector after patch for Bug 254665 (3.97 MB, video/mp4)
2023-03-31 10:49 PDT, Razvan Caliman
no flags Details
Screen recording of Web Inspector in Safari Technology Preview 173 (4.17 MB, video/mp4)
2023-07-07 07:01 PDT, Razvan Caliman
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Sephton 2023-03-28 07:15:48 PDT
Created attachment 465631 [details]
screen recording of issue

transform scale causes elements to be hidden even when style is overridden

Repro

1. Safari Extension sets style of its injected element (this style broken in Safari 16.4 but has worked for the past 6 years)

https://github.com/uetchy/Polyglot

#polyglot__panel {
	transform: scale(0) !important;
}

2. User CSS sets overriding styles

#polyglot__panel {
	transform: scale(1,1) !important;
}

3. Web Inspector shows correct style overriding
4. Element is still invisible
5. Style from extension needs to be activated by unchecking its line in the web inspector
Comment 1 Matt Sephton 2023-03-28 09:53:00 PDT
I'm testing this on: https://ja.wikipedia.org/wiki/モアイ
Comment 2 Razvan Caliman 2023-03-29 01:31:27 PDT
Created attachment 465655 [details]
test case

Reduced test case that exhibits the issue with animation end state being overridden by property with !important.
Comment 3 Radar WebKit Bug Importer 2023-03-29 01:43:10 PDT
<rdar://problem/107358080>
Comment 4 Razvan Caliman 2023-03-29 01:44:08 PDT
Thank you for filing this issue, Matt!

I did quick investigation. The panel doesn't stay visible because the end state of the animation defined by `@keyframes polyglot__envelope` gets overridden by the initial property which is marked with !important: 

```
#polyglot__panel {
   transform: scale(0) !important;
...
   animation: polyglot__envelope 0.3s cubic-bezier(0.175, 0.885, 0.32, 1.275)
              forwards !important;
}

@keyframes polyglot__envelope {
  0% {
    transform: scale(0);
  }
  100% {
    transform: scale(1);
  }
}
```

That is indeed a regression. I tested in Safari 15.1 and the attached test case [1] does work there while it fails in Safari 16.4.

I'm confused about the Web Inspector aspect of this.

How does the "User CSS" you describe in step 2 end up in the document?

>2. User CSS sets overriding styles

>#polyglot__panel {
>	transform: scale(1,1) !important;
>}


I looked into the public source of https://github.com/uetchy/Polyglot and can't see where this happens.

I manually added a stylesheet to the sample test case (commented-out in the source) and Web Inspector does list the correct order of overridden styles.

How can I get into the same state you're testing?

 
[1] https://bug-254585-attachments.webkit.org/attachment.cgi?id=465655
Comment 5 Razvan Caliman 2023-03-29 01:45:35 PDT
Created attachment 465656 [details]
test case screen recording

Screen recording of test case.
Comment 6 Razvan Caliman 2023-03-29 02:50:45 PDT
I discussed the issue about the animation to `transform: scale(1)` and then the immediate revert to `transform: scale(0) !important` with one of our animation engineers.

The animation *not maintaining* its end state is a bug fix, not a regression. 

In fact, the animation of the `transform` shouldn't run at all because of the `!important` on the original property. The fact that it did run in previous Safari versions was a bug.

There is still a fix to be made here: the animation still runs for accelerated properties like `transform`. It shouldn't run at all. It does not run for non-accelerated properties like `margin-left`.

This is consistent with Chrome and Firefox implementations.

---

@Matt, I'm still curious how the extra user CSS for `#polyglot__envelope` ends up in the stylesheet so I can debug the Web Inspector aspect of this.
Comment 7 Matt Sephton 2023-03-29 07:34:01 PDT
Created attachment 465659 [details]
how i apply user stylesheet
Comment 8 Matt Sephton 2023-03-29 07:36:58 PDT
Thanks for following up on this, it's very encouraging to see (compared to macOS and iOS bugs).

I had assumed it was a bug fix as the problematic CSS seemed wrong to me (I didn't write it), though I thought it best to check.

In answer to your question, I apply a user stylesheet through Safari Preferences > Advanced > Style sheet > Other... and pick a local file polyglot.css containing some revised CSS

```
div#polyglot__panel {
	transform: scale(1) !important;
}
```

I was not aware of polyglot__envelope until now, so I'll need to look into that.

See: https://github.com/uetchy/Polyglot/issues/128#issuecomment-1487229870
Comment 9 Matt Sephton 2023-03-29 07:41:20 PDT
Just to clarify, I only created the user stylesheet to workaround the issue.

It was not used when reproducing this issue or taking my screen recording!

HTH
Comment 10 Matt Sephton 2023-03-29 07:42:54 PDT
Created attachment 465661 [details]
my user stylesheet to workaround the issue
Comment 11 Matt Sephton 2023-03-29 08:03:34 PDT
OK, please ignore my previous message.

Detailed repro

0. Install Polyglot https://apps.apple.com/us/app/polyglot/id1471801525?mt=12
1. Safari Extension sets style of its injected element (this style broken in Safari 16.4 but has worked for the past 6 years = WebKit bug fix)
2. User CSS sets overriding styles, save the below CSS to a local file and apply using Safari Preferences > Advanced > Style sheet > Other...

#polyglot__panel {
	transform: scale(1) !important;
}

3. Web Inspector shows correct style overriding
4. Element is still invisible
5. Style from extension needs to be activated by unchecking its line in the web inspector

Please confirm you can repro.
Comment 12 Antoine Quint 2023-03-29 08:45:32 PDT
This uncovered an issue with animations of accelerated properties overridden by a static !important property which is tracked by bug 254665.
Comment 13 Razvan Caliman 2023-03-29 12:26:08 PDT
(In reply to Matt Sephton from comment #11) 

> Please confirm you can repro.

Thank you for the detailed steps to reproduce!

I confirm that I can reproduce the issue with mishandled overridden styles in Web Inspector on Safari Technology Preview 166.

I'm going to wait for the patch for Bug 254665 to land before I investigate further in case the sudden animation of the `!important` property trips Web Inspector.

---

To clarify, the issue fixed by Bug 254665 will make it so the `#polyglot__panel` will not show up anymore as it did in previous Safari versions. That's a bug fix. Bug 254590 will probably be closed as "works as expected" / not a regression.

Your workaround, or the removal of `!important` from  `transform: scale(0) !important;` will have to make it into the extension's source.
Comment 14 Matt Sephton 2023-03-29 16:05:17 PDT
Thanks for the summary. 

After the next fix, which causes the animation to not run at all, would the panel still be hidden?

The extension author is MIA, I'm suspecting the worst, so not sure what can be done. Forking would create its own headaches, so I'll probably leave it at the user stylesheet workaround.
Comment 15 Razvan Caliman 2023-03-30 06:17:32 PDT
(In reply to Matt Sephton from comment #14)
> After the next fix, which causes the animation to not run at all, would the
> panel still be hidden?

Yes, after that fix lands, the animation won't run at all and the panel will not be visible because of the `!important` property that wins in the end:

```
#polyglot__panel {
   transform: scale(0) !important;
}
```

The extension unfortunately relied on a WebKit bug that's being fixed now to bring it in line with other browser engines. The same code will not work in Chrome or Firefox either. 

I assume this extension was build only for Safari, otherwise the issue might have been spotted earlier.

The source of the extension will need to be updated to remove the `!important` from the `transform` property, or to include your override (and remove the animation altogether).
Comment 16 Razvan Caliman 2023-03-31 10:49:02 PDT
Created attachment 465708 [details]
Screen recording of Web Inspector after patch for Bug 254665

> I'm going to wait for the patch for Bug 254665 to land before I investigate further in case the sudden animation of the `!important` property trips Web Inspector.

@Matt, I made a local WebKit build with the patch from Bug 254665 and followed the steps to reproduce you shared.

I see Web Inspector showing the correct overridden state for CSS properties, including the one from the user stylesheet. Please see the attached recording.

At this point, I'm tempted to say the Web Inspector bug was related to the animation and it's now been fixed.

Once that patch ends up in Safari Technology Preview, I will test again and ask you to do the same. 

I'll keep this bug open until then as a reminder.

If you find steps to reproduce this issue with Web Inspector's faulty listing of overridden properties independent from this issue with the extension, please do share.

Thanks again for taking the time to file the issue! 
It helped surface that Bug 254665 needed fixing.
Comment 17 Razvan Caliman 2023-04-03 10:50:01 PDT
(In reply to Matt Sephton from comment #14)
> The extension author is MIA, I'm suspecting the worst, so not sure what can
> be done.

Matt, were you able to get in touch with the extension's owner to publish an updated version?
Comment 18 Matt Sephton 2023-05-19 06:24:56 PDT
Sadly not. The author had health problems so I can only assume the worst - that they have died.

In other news, to try to lighten the mood, I have filed a new bug of a similar type in Safari 16.5 that is also in Safari Tech Preview 170
https://bugs.webkit.org/show_bug.cgi?id=257023
Comment 19 Razvan Caliman 2023-07-07 07:01:56 PDT
Created attachment 466975 [details]
Screen recording of Web Inspector in Safari Technology Preview 173

Hi Matt, 

Following up: I tested in the latest Safari Technology Preview 173 and the issue with Web Inspector no longer reproduces. The fix for Bug 254665 solved it.

I'm going to close this issue now.