[web] Remove non-ShadowDom mode

[mdebbar]

If we still want to do this, here's a quick PR :)

Fixes flutter/flutter#116204

[ditman]

This cleanup is great. Thanks for adding the throw "this won't work without ShadowDOM" in the embedder, it makes a lot of sense.

Also, millions of thanks for creating the global_styles.dart file. 😭

I think some of the tests can be simplified and moved to a global_styles_test.dart file and tested there in more isolation, but the rest looks good, especially if all the tests continue to pass :P

(This is going to cause some conflicts with @htoor3's incoming PR though!)

[ditman]

Thanks for this test!

[ditman]

I'd move these tests to a separate global_styles_test.dart where we assert the things that must be contained in the sheet, then in the embedder_test leave only the ones that assert that a sheet was added at the right spot (hasCssRule shouldn't live in embedder_test.dart, but in global_stles_test.dart, IMO)

[mdebbar]

Done

[ditman]

Safari is being fun and failing here:

00:04 �[32m+182�[0m�[33m ~11�[0m�[31m -1�[0m: test/embedder_test.dart: Shadow root and styles Attaches outrageous text styles to flt-scene-host �[1m�[31m[E]�[0m�[0m                                                                               
  Expected: true
    Actual: <false>
  Should pass default css font.
  
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart 1142:37                             wrapException
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_helper.dart 1173:25                             throwExpression

[mdebbar]

I think some of the tests can be simplified and moved to a global_styles_test.dart file and tested there in more isolation, but the rest looks good, especially if all the tests continue to pass :P

Great idea! It'll help us fix the Safari failure too (I'll explain below).

(This is going to cause some conflicts with @htoor3's incoming PR though!)

Yep. I plan to wait for @htoor3 to land his PR first.

Safari is being fun and failing here:

Ugh. This is because we set the default css font to font: normal normal 14px sans-serif. But browsers take that string and process it however they like. Chrome, for example, removes the normal normal part because it's the default (normal normal 14px sans-serif == 14px sans-serif). I'm guessing Safari doesn't do that 🤷‍♂️

Your idea of moving some tests to global_styles_test.dart will solve this issue because I can go back to passing a simpler default css font (14px monospace) like you were doing here:

engine/lib/web_ui/test/engine/host_node_test.dart

Line 18 in 37ff1ca

final HostNode hostNode = ShadowDomHostNode(rootNode, '14px monospace');

to avoid the normal normal thingy.

[ditman]

Chrome, for example, removes the normal normal part because it's the default (normal normal 14px sans-serif == 14px sans-serif). I'm guessing Safari doesn't do that

What I remember from Safari (not sure if from this change or one from @htoor3) was that they'd unroll the shorthand CSS property into every separate property, so if you did: font: 14px monospace, in safari you'd need to check: font-size: 14px and font-family: monospace, rather than what you had set. 🙈‼️

[mdebbar]

@ditman @htoor3 this is ready for another review after rebasing on top of the @htoor3's restructuring PR.

[ditman]

This LGTM! Thank you very much for the cleanup @mdebbar!