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

Revert "[web] Move text editing nodes outside of shadowDOM" #40847

Merged
merged 1 commit into from
Apr 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading