Polyfill legacy offsetParent behavior #156
Conversation
The spec for offsetParent was changed here: w3c/csswg-drafts#159 This change was implemented in Safari and Firefox, but hasn't been implemented in Chrome, until now: https://chromium-review.googlesource.com/c/chromium/src/+/2775208 When I made this change in chrome, it broke some chrome:// internal pages which use paper-tooltip: https://bugs.chromium.org/p/chromium/issues/detail?id=1200750 https://bugs.chromium.org/p/chromium/issues/detail?id=1202105 This patch fixes paper-tooltip to use the new offsetParent behavior by adding a polyfill for the old offsetParent behavior. The issues reported in the above bugs would also occur in Firefox and Safari, but nobody ever found out because those chrome:// pages were obviously never tested in Firefox or Safari.
|
It turns out that chromium already has a patch system that I can use to patch this behavior into chrome in order to fix the chrome:// copy of paper-tooltip of interest, so I'll just do that. |
|
(relaying from chat) Sounds like Chromium isn't interested in keeping a local patch, so we're going to do a regular release for this, which I think is safe since this should be fixing the element for an intentionally breaking change in Chromium. http://cl/387845879 is the CL I'm going to use for testing. @josepharhar, I think you also said this would fix the same issue as #155? |
|
After reading #155 I think they are separate unrelated fixes |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
1 similar comment
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
josepharhar
force-pushed
the
offsetparent
branch
from
9116f0d
to
5ee9f62
Compare
When I first changed the behavior of offsetParent for crbug.com/920069 it broke some webui pages which used paper-toolip: crbug.com/1200750 I minimized the broken case and determined that paper-tooltip was broken. I made a PR on paper-tooltip here, but it's unlikely that it will ever get merged: PolymerElements/paper-tooltip#156 The fix to paper-tooltip is to polyfill the old offsetParent behavior in javascript. This patch applies the fix to paper-tooltip within chrome and re-enables the new offsetParent behavior. Fixed: 920069 Change-Id: Ib39c34a6c15b44161a7942c7ee9cfd4afe68acf7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061435 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: dpapad <[email protected]> Cr-Commit-Position: refs/heads/main@{#935992}
This reverts commit def5070. Reason for revert: The polyfill I wrote, not the actual offsetParent flag, broke something else. I'll need to figure out what I did wrong in the polyfill and fix it in a reland: crbug.com/1264786 Fixed: 1264786 Original change's description: > Re-enable offsetParent and fix in webui's paper-tooltip > > When I first changed the behavior of offsetParent for crbug.com/920069 > it broke some webui pages which used paper-toolip: crbug.com/1200750 > > I minimized the broken case and determined that paper-tooltip was > broken. I made a PR on paper-tooltip here, but it's unlikely that it > will ever get merged: > PolymerElements/paper-tooltip#156 > > The fix to paper-tooltip is to polyfill the old offsetParent behavior in > javascript. This patch applies the fix to paper-tooltip within chrome > and re-enables the new offsetParent behavior. > > Fixed: 920069 > Change-Id: Ib39c34a6c15b44161a7942c7ee9cfd4afe68acf7 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061435 > Commit-Queue: Joey Arhar <[email protected]> > Reviewed-by: dpapad <[email protected]> > Cr-Commit-Position: refs/heads/main@{#935992} (cherry picked from commit 5c36b4e) Change-Id: I27faaeb162a004130e4692595530ea5ab4300574 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3280437 Reviewed-by: Joey Arhar <[email protected]> Reviewed-by: dpapad <[email protected]> Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: dpapad <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#941603} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3282903 Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/branch-heads/4692@{chromium#200} Cr-Branched-From: 038cd96-refs/heads/main@{#938553}
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}
When I first changed the behavior of offsetParent for crbug.com/920069 it broke some webui pages which used paper-toolip: crbug.com/1200750 I minimized the broken case and determined that paper-tooltip was broken. I made a PR on paper-tooltip here, but it's unlikely that it will ever get merged: PolymerElements/paper-tooltip#156 The fix to paper-tooltip is to polyfill the old offsetParent behavior in javascript. This patch applies the fix to paper-tooltip within chrome and re-enables the new offsetParent behavior. Fixed: 920069 Change-Id: Ib39c34a6c15b44161a7942c7ee9cfd4afe68acf7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061435 Commit-Queue: Joey Arhar <[email protected]> Reviewed-by: dpapad <[email protected]> Cr-Commit-Position: refs/heads/main@{#935992} NOKEYCHECK=True GitOrigin-RevId: def507007a4d56a61c433f565c1f0beac7790831
This reverts commit def507007a4d56a61c433f565c1f0beac7790831. Reason for revert: The polyfill I wrote, not the actual offsetParent flag, broke something else. I'll need to figure out what I did wrong in the polyfill and fix it in a reland: crbug.com/1264786 Fixed: 1264786 Original change's description: > Re-enable offsetParent and fix in webui's paper-tooltip > > When I first changed the behavior of offsetParent for crbug.com/920069 > it broke some webui pages which used paper-toolip: crbug.com/1200750 > > I minimized the broken case and determined that paper-tooltip was > broken. I made a PR on paper-tooltip here, but it's unlikely that it > will ever get merged: > PolymerElements/paper-tooltip#156 > > The fix to paper-tooltip is to polyfill the old offsetParent behavior in > javascript. This patch applies the fix to paper-tooltip within chrome > and re-enables the new offsetParent behavior. > > Fixed: 920069 > Change-Id: Ib39c34a6c15b44161a7942c7ee9cfd4afe68acf7 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3061435 > Commit-Queue: Joey Arhar <[email protected]> > Reviewed-by: dpapad <[email protected]> > Cr-Commit-Position: refs/heads/main@{#935992} Change-Id: I27faaeb162a004130e4692595530ea5ab4300574 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3280437 Reviewed-by: Joey Arhar <[email protected]> Reviewed-by: dpapad <[email protected]> Auto-Submit: Joey Arhar <[email protected]> Commit-Queue: dpapad <[email protected]> Cr-Commit-Position: refs/heads/main@{#941603} NOKEYCHECK=True GitOrigin-RevId: 5c36b4e8268c42e6d206af77efe45d58419043fe
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
The spec for offsetParent was changed here:
w3c/csswg-drafts#159
This change was implemented in Safari and Firefox, but hasn't been
implemented in Chrome, until now:
https://chromium-review.googlesource.com/c/chromium/src/+/2775208
When I made this change in chrome, it broke some chrome:// internal
pages which use paper-tooltip:
https://bugs.chromium.org/p/chromium/issues/detail?id=1200750
https://bugs.chromium.org/p/chromium/issues/detail?id=1202105
This patch fixes paper-tooltip to use the new offsetParent behavior by
adding a polyfill for the old offsetParent behavior. The issues reported
in the above bugs would also occur in Firefox and Safari, but nobody
ever found out because those chrome:// pages were obviously never tested
in Firefox or Safari.