Skip to content

Commit

Permalink
Reland "Re-enable offsetParent and fix in webui's paper-tooltip"
Browse files Browse the repository at this point in the history
Original patch: http://crrev.com/872658
First revert/disable: http://crrev.com/879028
Second patch: http://crrev.com/935992
Second revert/disable: http://crrev.com/941603

The second attempt at this failed because the polyfill I wrote was
modifying the display property of nodes while computing offsetParent. I
rewrote the polyfill and verified that both of the bugs causing
disables/reverts are still fixed.

I made a pull request to add the polyfill upstream, but the upstream
maintainers will not publish any new releases and would prefer not to
merge any new PRs:
PolymerElements/paper-tooltip#156

Fixed: 920069
Change-Id: I006fec2d88da05dd4c3e2b2aa55352c02105fac0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3566126
Reviewed-by: Mason Freed <[email protected]>
Reviewed-by: Demetrios Papadopoulos <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#989056}
NOKEYCHECK=True
GitOrigin-RevId: 600466d536907bc751f2a8ea1687cd7fec0e02e3
  • Loading branch information
josepharhar authored and copybara-github committed Apr 5, 2022
1 parent 3add051 commit 4d2d1e9
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 4 deletions.
2 changes: 1 addition & 1 deletion blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ const base::FeatureParam<int> kCacheCodeOnIdleDelayParam{&kCacheCodeOnIdle,
// TODO(crbug.com/920069): Remove this once the feature has
// landed and no compat issues are reported.
const base::Feature kOffsetParentNewSpecBehavior{
"OffsetParentNewSpecBehavior", base::FEATURE_DISABLED_BY_DEFAULT};
"OffsetParentNewSpecBehavior", base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kKeepScriptResourceAlive{"KeepScriptResourceAlive",
base::FEATURE_DISABLED_BY_DEFAULT};
Expand Down
2 changes: 1 addition & 1 deletion blink/renderer/platform/runtime_enabled_features.json5
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,7 @@
// TODO(crbug.com/920069): Remove OffsetParentNewSpecBehavior after the
// feature is in stable with no issues.
name: "OffsetParentNewSpecBehavior",
status: "experimental"
status: "stable"
},
{
name: "OnDeviceChange",
Expand Down
69 changes: 69 additions & 0 deletions polymer/v3_0/chromium.patch
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,72 @@ index 6af2fa359336..c56a6737e1d8 100644
--paper-grey-50: #fafafa;
--paper-grey-100: #f5f5f5;
--paper-grey-200: #eeeeee;
diff --git a/components-chromium/paper-tooltip/paper-tooltip.js b/components-chromium/paper-tooltip/paper-tooltip.js
index 303d1bbdfc787..4adc8ea38d760 100644
--- a/components-chromium/paper-tooltip/paper-tooltip.js
+++ b/components-chromium/paper-tooltip/paper-tooltip.js
@@ -442,13 +442,16 @@ Polymer({
* @return {void}
*/
updatePosition: function() {
- if (!this._target || !this.offsetParent)
+ if (!this._target)
+ return;
+ var offsetParent = this._composedOffsetParent();
+ if (!offsetParent)
return;
var offset = this.offset;
// If a marginTop has been provided by the user (pre 1.0.3), use it.
if (this.marginTop != 14 && this.offset == 14)
offset = this.marginTop;
- var parentRect = this.offsetParent.getBoundingClientRect();
+ var parentRect = offsetParent.getBoundingClientRect();
var targetRect = this._target.getBoundingClientRect();
var thisRect = this.getBoundingClientRect();
var horizontalCenterOffset = (targetRect.width - thisRect.width) / 2;
@@ -594,5 +597,45 @@ Polymer({
}
this.unlisten(this.$.tooltip, 'animationend', '_onAnimationEnd');
this.unlisten(this, 'mouseenter', 'hide');
+ },
+
+ /**
+ * Polyfills the old offsetParent behavior from before the spec was changed:
+ * https://github.com/w3c/csswg-drafts/issues/159
+ */
+ _composedOffsetParent: function() {
+ // Do an initial walk to check for display:none ancestors.
+ for (let ancestor = this; ancestor; ancestor = flatTreeParent(ancestor)) {
+ if (!(ancestor instanceof Element))
+ continue;
+ if (getComputedStyle(ancestor).display === 'none')
+ return null;
+ }
+
+ for (let ancestor = flatTreeParent(this); ancestor; ancestor = flatTreeParent(ancestor)) {
+ if (!(ancestor instanceof Element))
+ continue;
+ const style = getComputedStyle(ancestor);
+ if (style.display === 'contents') {
+ // display:contents nodes aren't in the layout tree so they should be skipped.
+ continue;
+ }
+ if (style.position !== 'static') {
+ return ancestor;
+ }
+ if (ancestor.tagName === 'BODY')
+ return ancestor;
+ }
+ return null;
+
+ function flatTreeParent(element) {
+ if (element.assignedSlot) {
+ return element.assignedSlot;
+ }
+ if (element.parentNode instanceof ShadowRoot) {
+ return element.parentNode.host;
+ }
+ return element.parentNode;
+ }
}
});
47 changes: 45 additions & 2 deletions polymer/v3_0/components-chromium/paper-tooltip/paper-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,16 @@ Polymer({
* @return {void}
*/
updatePosition: function() {
if (!this._target || !this.offsetParent)
if (!this._target)
return;
var offsetParent = this._composedOffsetParent();
if (!offsetParent)
return;
var offset = this.offset;
// If a marginTop has been provided by the user (pre 1.0.3), use it.
if (this.marginTop != 14 && this.offset == 14)
offset = this.marginTop;
var parentRect = this.offsetParent.getBoundingClientRect();
var parentRect = offsetParent.getBoundingClientRect();
var targetRect = this._target.getBoundingClientRect();
var thisRect = this.getBoundingClientRect();
var horizontalCenterOffset = (targetRect.width - thisRect.width) / 2;
Expand Down Expand Up @@ -594,5 +597,45 @@ Polymer({
}
this.unlisten(this.$.tooltip, 'animationend', '_onAnimationEnd');
this.unlisten(this, 'mouseenter', 'hide');
},

/**
* Polyfills the old offsetParent behavior from before the spec was changed:
* https://github.com/w3c/csswg-drafts/issues/159
*/
_composedOffsetParent: function() {
// Do an initial walk to check for display:none ancestors.
for (let ancestor = this; ancestor; ancestor = flatTreeParent(ancestor)) {
if (!(ancestor instanceof Element))
continue;
if (getComputedStyle(ancestor).display === 'none')
return null;
}

for (let ancestor = flatTreeParent(this); ancestor; ancestor = flatTreeParent(ancestor)) {
if (!(ancestor instanceof Element))
continue;
const style = getComputedStyle(ancestor);
if (style.display === 'contents') {
// display:contents nodes aren't in the layout tree so they should be skipped.
continue;
}
if (style.position !== 'static') {
return ancestor;
}
if (ancestor.tagName === 'BODY')
return ancestor;
}
return null;

function flatTreeParent(element) {
if (element.assignedSlot) {
return element.assignedSlot;
}
if (element.parentNode instanceof ShadowRoot) {
return element.parentNode.host;
}
return element.parentNode;
}
}
});

0 comments on commit 4d2d1e9

Please sign in to comment.