Skip to content

Commit

Permalink
Revert "[web] Move text editing nodes outside of shadowDOM (#39688)"
Browse files Browse the repository at this point in the history
This reverts commit 0416103.
  • Loading branch information
zanderso committed Apr 1, 2023
1 parent b1ba58f commit a6c1c4c
Show file tree
Hide file tree
Showing 12 changed files with 165 additions and 192 deletions.
51 changes: 12 additions & 39 deletions lib/web_ui/lib/src/engine/embedder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,6 @@ class FlutterViewEmbedder {
HostNode get glassPaneShadow => _glassPaneShadow;
late HostNode _glassPaneShadow;

DomElement get textEditingHostNode => _textEditingHostNode;
late DomElement _textEditingHostNode;

static const String defaultFontStyle = 'normal';
static const String defaultFontWeight = 'normal';
static const double defaultFontSize = 14;
Expand Down Expand Up @@ -171,9 +168,6 @@ class FlutterViewEmbedder {
);
_glassPaneShadow = glassPaneElementHostNode;

_textEditingHostNode =
createTextEditingHostNode(glassPaneElement, defaultCssFont);

// Don't allow the scene to receive pointer events.
_sceneHostElement = domDocument.createElement('flt-scene-host')
..style.pointerEvents = 'none';
Expand All @@ -195,19 +189,19 @@ class FlutterViewEmbedder {
glassPaneElementHostNode.appendAll(<DomNode>[
accessibilityPlaceholder,
_sceneHostElement!,
]);

// The semantic host goes last because hit-test order-wise it must be
// first. If semantics goes under the scene host, platform views will
// obscure semantic elements.
//
// You may be wondering: wouldn't semantics obscure platform views and
// make then not accessible? At least with some careful planning, that
// should not be the case. The semantics tree makes all of its non-leaf
// elements transparent. This way, if a platform view appears among other
// interactive Flutter widgets, as long as those widgets do not intersect
// with the platform view, the platform view will be reachable.
glassPaneElement.appendChild(semanticsHostElement);
// The semantic host goes last because hit-test order-wise it must be
// first. If semantics goes under the scene host, platform views will
// obscure semantic elements.
//
// You may be wondering: wouldn't semantics obscure platform views and
// make then not accessible? At least with some careful planning, that
// should not be the case. The semantics tree makes all of its non-leaf
// elements transparent. This way, if a platform view appears among other
// interactive Flutter widgets, as long as those widgets do not intersect
// with the platform view, the platform view will be reachable.
semanticsHostElement,
]);

// When debugging semantics, make the scene semi-transparent so that the
// semantics tree is more prominent.
Expand Down Expand Up @@ -399,24 +393,3 @@ FlutterViewEmbedder? _flutterViewEmbedder;
FlutterViewEmbedder ensureFlutterViewEmbedderInitialized() =>
_flutterViewEmbedder ??=
FlutterViewEmbedder(hostElement: configuration.hostElement);

/// Creates a node to host text editing elements and applies a stylesheet
/// to Flutter nodes that exist outside of the shadowDOM.
DomElement createTextEditingHostNode(DomElement root, String defaultFont) {
final DomElement domElement =
domDocument.createElement('flt-text-editing-host');
final DomHTMLStyleElement styleElement = createDomHTMLStyleElement();

styleElement.id = 'flt-text-editing-stylesheet';
root.appendChild(styleElement);
applyGlobalCssRulesToSheet(
styleElement.sheet! as DomCSSStyleSheet,
hasAutofillOverlay: browserHasAutofillOverlay(),
cssSelectorPrefix: FlutterViewEmbedder.glassPaneTagName,
defaultCssFont: defaultFont,
);

root.appendChild(domElement);

return domElement;
}
17 changes: 5 additions & 12 deletions lib/web_ui/lib/src/engine/host_node.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ abstract class HostNode {
/// See:
/// * [Document.querySelectorAll](https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelectorAll)
Iterable<DomElement> querySelectorAll(String selectors);

DomElement get renderHost;
}

/// A [HostNode] implementation, backed by a [DomShadowRoot].
Expand All @@ -112,10 +110,11 @@ class ShadowDomHostNode implements HostNode {
/// This also calls [applyGlobalCssRulesToSheet], with the [defaultFont]
/// to be used as the default font definition.
ShadowDomHostNode(DomElement root, String defaultFont)
: assert(root.isConnected ?? true,
'The `root` of a ShadowDomHostNode must be connected to the Document object or a ShadowRoot.') {
root.appendChild(renderHost);
_shadow = renderHost.attachShadow(<String, dynamic>{
: assert(
root.isConnected ?? true,
'The `root` of a ShadowDomHostNode must be connected to the Document object or a ShadowRoot.'
) {
_shadow = root.attachShadow(<String, dynamic>{
'mode': 'open',
// This needs to stay false to prevent issues like this:
// - https://github.com/flutter/flutter/issues/85759
Expand All @@ -136,9 +135,6 @@ class ShadowDomHostNode implements HostNode {

late DomShadowRoot _shadow;

@override
final DomElement renderHost = domDocument.createElement('flt-render-host');

@override
DomElement? get activeElement => _shadow.activeElement;

Expand Down Expand Up @@ -195,9 +191,6 @@ class ElementHostNode implements HostNode {

late DomElement _element;

@override
final DomElement renderHost = domDocument.createElement('flt-render-host');

@override
DomElement? get activeElement => _element.ownerDocument?.activeElement;

Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/platform_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
_platformViewMessageHandler ??= PlatformViewMessageHandler(
contentManager: platformViewManager,
contentHandler: (DomElement content) {
flutterViewEmbedder.glassPaneShadow.renderHost.append(content);
flutterViewEmbedder.glassPaneElement.append(content);
},
);
_platformViewMessageHandler!.handlePlatformViewCall(data, callback!);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,8 @@ class PlatformViewManager {
}

_ensureContentCorrectlySized(content, viewType);
wrapper.append(content);

return wrapper;
return wrapper..append(content);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,20 @@ import '../semantics.dart' show EngineSemanticsOwner;
/// It also takes into account semantics being enabled to fix the case where
/// offsetX, offsetY == 0 (TalkBack events).
ui.Offset computeEventOffsetToTarget(DomMouseEvent event, DomElement actualTarget) {
// On top of a platform view
if (event.target != actualTarget) {
return _computeOffsetOnPlatformView(event, actualTarget);
}
// On a TalkBack event
if (EngineSemanticsOwner.instance.semanticsEnabled && event.offsetX == 0 && event.offsetY == 0) {
return _computeOffsetForTalkbackEvent(event, actualTarget);
}

final bool isTargetOutsideOfShadowDOM = event.target != actualTarget;
if (isTargetOutsideOfShadowDOM) {
return _computeOffsetRelativeToActualTarget(event, actualTarget);
}
// Return the offsetX/Y in the normal case.
// (This works with 3D translations of the parent element.)
return ui.Offset(event.offsetX, event.offsetY);
}

/// Computes the event offset when hovering over any nodes that don't exist in
/// the shadowDOM such as platform views or text editing nodes.
/// Computes the event offset when hovering over a platformView.
///
/// This still uses offsetX/Y, but adds the offset from the top/left corner of the
/// platform view to the glass pane (`actualTarget`).
Expand All @@ -59,7 +57,7 @@ ui.Offset computeEventOffsetToTarget(DomMouseEvent event, DomElement actualTarge
///
/// Event offset relative to FlutterView = (offsetX + xP, offsetY + yP)
// TODO(dit): Make this understand 3D transforms, https://github.com/flutter/flutter/issues/117091
ui.Offset _computeOffsetRelativeToActualTarget(DomMouseEvent event, DomElement actualTarget) {
ui.Offset _computeOffsetOnPlatformView(DomMouseEvent event, DomElement actualTarget) {
final DomElement target = event.target! as DomElement;
final DomRect targetRect = target.getBoundingClientRect();
final DomRect actualTargetRect = actualTarget.getBoundingClientRect();
Expand Down
5 changes: 3 additions & 2 deletions lib/web_ui/lib/src/engine/semantics/text_field.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'package:ui/ui.dart' as ui;

import '../browser_detection.dart';
import '../dom.dart';
import '../embedder.dart';
import '../platform_dispatcher.dart';
import '../safe_browser_api.dart';
import '../text_editing/text_editing.dart';
Expand Down Expand Up @@ -421,14 +422,14 @@ class TextField extends RoleManager {
..height = '${semanticsObject.rect!.height}px';

if (semanticsObject.hasFocus) {
if (domDocument.activeElement !=
if (flutterViewEmbedder.glassPaneShadow.activeElement !=
activeEditableElement) {
semanticsObject.owner.addOneTimePostUpdateCallback(() {
activeEditableElement.focus();
});
}
SemanticsTextEditingStrategy._instance?.activate(this);
} else if (domDocument.activeElement ==
} else if (flutterViewEmbedder.glassPaneShadow.activeElement ==
activeEditableElement) {
if (!isIosSafari) {
SemanticsTextEditingStrategy._instance?.deactivate(this);
Expand Down
3 changes: 1 addition & 2 deletions lib/web_ui/lib/src/engine/text_editing/text_editing.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ void _emptyCallback(dynamic _) {}

/// The default [HostNode] that hosts all DOM required for text editing when a11y is not enabled.
@visibleForTesting
DomElement get defaultTextEditingRoot =>
flutterViewEmbedder.textEditingHostNode;
HostNode get defaultTextEditingRoot => flutterViewEmbedder.glassPaneShadow;

/// These style attributes are constant throughout the life time of an input
/// element.
Expand Down
9 changes: 4 additions & 5 deletions lib/web_ui/test/engine/host_node_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,19 @@ void testMain() {

group('ShadowDomHostNode', () {
final HostNode hostNode = ShadowDomHostNode(rootNode, '14px monospace');
final DomElement renderHost = domDocument.querySelector('flt-render-host')!;

test('Initializes and attaches a shadow root', () {
expect(domInstanceOfString(hostNode.node, 'ShadowRoot'), isTrue);
expect((hostNode.node as DomShadowRoot).host, renderHost);
expect(hostNode.node, renderHost.shadowRoot);
expect((hostNode.node as DomShadowRoot).host, rootNode);
expect(hostNode.node, rootNode.shadowRoot);

// The shadow root should be initialized with correct parameters.
expect(renderHost.shadowRoot!.mode, 'open');
expect(rootNode.shadowRoot!.mode, 'open');
if (browserEngine != BrowserEngine.firefox &&
browserEngine != BrowserEngine.webkit) {
// Older versions of Safari and Firefox don't support this flag yet.
// See: https://caniuse.com/mdn-api_shadowroot_delegatesfocus
expect(renderHost.shadowRoot!.delegatesFocus, isFalse);
expect(rootNode.shadowRoot!.delegatesFocus, isFalse);
}
});

Expand Down

0 comments on commit a6c1c4c

Please sign in to comment.