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

[Bug]: Edge case where specific element does not receive click as expected in Electron 29.0.0 and above #42214

Closed
3 tasks done
Nantris opened this issue May 16, 2024 · 15 comments

Comments

@Nantris
Copy link
Contributor

Nantris commented May 16, 2024

Preflight Checklist

Electron Version

29.0.0-alpha.8 and above

What operating system are you using?

Other Linux

Operating System Version

Kubuntu 23.04/Windows 11

What arch are you using?

x64

Last Known Working Electron version

29.0.0-alpha.7

Expected Behavior

Clicking into <pre> element with white-space: pre inside of text editor should work.

Actual Behavior

Clicks are largely ignored in that one sort of element, although not in any others, and not if white-space: pre is removed. This does not appear to be explainable by any traditional explanation such as zIndex, invisible elements, cancelled events, or otherwise.

Testcase Gist URL

I have not been able to reproduce this, but am quite certain this edge case exists

Additional Information

Our code runs perfectly in Electron 28.3.1, and in Chrome 124 (which is in theory basically identical to Electron 29.3.x) - and yet the code that fails in Electron works fine in the browser. Affects Linux and Windows both, and disabling GPU acceleration or using Vulkan does not change this.

I removed all CSS and DOM besides the problem elements and the issue persists.

I load the exact same page in Chrome 124 or Electron 29.x and in Chrome it works fine whereas in Electron clicks are entirely ignored.

Although I cannot create a repro (despite having spent hours trying) this issue is plaguing us and has so far burned 4-5 entire workdays. I would be happy to demo it for a member of the Electron team in a web meeting, and I wonder if anyone has any theories as to what could possibly cause such a strange issue?

@Nantris
Copy link
Contributor Author

Nantris commented May 16, 2024

Specifically this issue begins in 29.0.0-alpha.8 and was not present in 29.0.0-alpha.7

v29.0.0-alpha.7...v29.0.0-alpha.8

@ckerr do you think your commit during that time could be related? Otherwise it would have to be upgrading Chromium that caused this, but similar versions of Chromium seemingly had no problems when using Chrome instead of Electron so I'm doubtful of that. The latest version of Chrome here, 124.0.6367.201 continues to not exhibit the issue, although Electron 30 (based upon 124.0.6367.207) does exhibit that issue still.

@Nantris
Copy link
Contributor Author

Nantris commented May 16, 2024

There are apparently no nightly builds between those versions so I cannot pinpoint the issue more precisely, but I feel pretty confident it must somehow be introduced in #40887.

@Nantris
Copy link
Contributor Author

Nantris commented May 17, 2024

If there's anything more I can do to help troubleshoot this issue - this is a 100% blocker issue for us. I spent 4-5 unhappy days tracking it down.

@Nantris
Copy link
Contributor Author

Nantris commented May 18, 2024

Working in Electron 28 (and 29.0.0-alpha.7)

_codeblock-working-electron-28.mp4

Broken in Electron 29 (version 29.0.0-alpha.8 and above)

_codeblock-bug-electron-29.mp4

You can see one click does get through in Electron 29. It almost seems like there is one point that sometimes still allows a click to focus the element. I'd like to emphasize that there is no plausible explanation for this issue except that it's a bug in Electron, since it works fine in Chrome based on the same Chromium version.

@Nantris
Copy link
Contributor Author

Nantris commented May 18, 2024

I tested in Chromium (instead of Chrome) 124.0.6367.118 and indeed the issue is not present.

@codebytere, sorry to ping you directly. I wonder if you have any thoughts on what change between these versions might be the cause? This is a total blocker for us and Electron 28 is reaching its end of life in just 3 weeks.

@Nantris
Copy link
Contributor Author

Nantris commented May 20, 2024

Friendly bump. I know this sounds like a crazy and misguided issue, but it really is legitimate. I hope someone can respond or else our project is in serious trouble.

@Nantris
Copy link
Contributor Author

Nantris commented May 21, 2024

Is there any way to access a nightly or commit-specific build so I can definitively determine which of those issues is likely the cause? I looked through most (but not yet all) of the Chromium changelog between the versions that alpha.7 and alpha.8 are based on and only found one very weak candidate for a possible cause, and meanwhile it continues to run fine in Chromium so it's hard to believe that any of the changes are the issue.

@clavin
Copy link
Member

clavin commented May 21, 2024

Hello! Thanks for taking the time to report this issue and investigate what change might have caused this behavior. Pinpointing the behavior change to between 29.0.0-alpha.7 and 29.0.0-alpha.8 is super helpful to us. From my experience, the Chromium upgrade in that range is almost definitely the culprit, but that's about all we know.

Unfortunately, we can't help resolve this bug without the ability to reproduce & investigate the issue ourselves. ☹️ I know you've poured time into trying to create one without much success isolating the issue on its own. I'm marking this issue blocked for right now as part of our process for triaging and prioritizing actionable issues, but if you are able to narrow the issue down to a small, independent bit of code, please do respond and add that information here! Also feel free to add more details as you discover more about this issue.

@clavin clavin added the blocked/need-repro Needs a test case to reproduce the bug label May 21, 2024
@electron-issue-triage
Copy link

Hello @Nantris. Thanks for reporting this and helping to make Electron better!

Would it be possible for you to make a standalone testcase with only the code necessary to reproduce the issue? For example, Electron Fiddle is a great tool for making small test cases and makes it easy to publish your test case to a gist that Electron maintainers can use.

Stand-alone test cases make fixing issues go more smoothly: it ensure everyone's looking at the same issue, it removes all unnecessary variables from the equation, and it can also provide the basis for automated regression tests.

Now adding the blocked/need-repro Needs a test case to reproduce the bug label for this reason. After you make a test case, please link to it in a followup comment. This issue will be closed in 10 days if the above is not addressed.

@Nantris
Copy link
Contributor Author

Nantris commented May 21, 2024

@clavin thanks very much for your attention on this!

I understand your strong suspicion that the bug is caused by the Chromium version upgrade rather than the other PR, but would it be possible for someone with expertise to take a look over the other PR that could be at fault? (#40887) The surface area of that single PR versus thousands of Chromium PRs makes it the easiest actionable approach in my opinion, although unfortunately not actionable by myself alone since I lack the relevant expertise. I understand you view this as a low likelihood, but it would be an enormous help to me in my attempts to isolate the problem.

I wonder if you have any thoughts on the fact that it doesn't affect Chromium/Chrome versions that I have tested (124/125) even though it does affect Electron builds based on those versions of Chromium? This to me is the biggest incongruity if, in fact, it is a Chromium bug despite these findings.

As mentioned, I have not found any way to make a standalone repro despite spending hours trying both to "pare down" my existing project into a minimum repo AND many hours more trying to "build up" a repro from scratch. I've been working on this nonstop for over a week from every conceivable angle without even the slightest clue ☹️

Thank you very much again for taking the time to read over my issue report; it is greatly appreciated.


PS: might I also propose that in the future, Chromium version upgrades be the sole commit for their version bumps (at least/especially in alpha/beta versions) to facilitate easier issue reporting and root cause analysis?

@electron-issue-triage electron-issue-triage bot removed the blocked/need-repro Needs a test case to reproduce the bug label May 21, 2024
@clavin
Copy link
Member

clavin commented May 21, 2024

I did in fact take a peek at #40887 while triaging this issue and I'm still confident that it didn't cause this bug. My rationale is that the changes in that PR have almost no effect1 on the execution of Electron, only the code we write.

If you don't mind, let me explain our strategy for upgrading Chromium. Chromium makes around 3000 commits every week (based on a quick commit count & average over 2024 so far). It's our job on the Upgrades Working Group to ingest that huge amount of code changes into Electron. It's thousands of people working on Chromium and a handful of folks working on upgrading Electron, so we're often sprinting to keep pace with Chromium. (I'm working on one of these upgrades right now haha.)

In those thousands of commits, Chromium may (and often does) change its code in ways that are incompatible with the old code, sometimes dramatically so, and we have to figure out how to keep Electron working the same with their new designs and systems. This is why a Chromium upgrade in Electron may also include many changes to Electron code: those changes are necessary to keep Electron working after bumping the Chromium version.

To clarify, I don't believe this is a Chromium bug right now. Electron heavily uses Chromium, but we have many patches and code on top of Chromium. As explained above, upgrading Chromium in Electron requires making changes to Electron. Thus, a Chromium upgrade can unfortunately create Electron bugs.

We desire that every commit to our release and mainline development branches to be self-contained and always build and pass tests. This gives contributors a solid and reliable base to start working on new changes. I hope the explanation above also shows why we can't only bump the version of Chromium without breaking other parts of the project and violating that desire.

That said, we do structure our Chromium upgrade PRs in a fashion like you are alluding to: a given commit is self-contained and either only updates the version of Chromium or makes only a specific change in response to a change in Chromium. On top of this, we also bump Chromium in these rolls in daily increments (unless Chromium has given us too much work in a short period, in which case we pause the automatic daily bump). Since Chromium can break things faster than we can fix them, it's rarely possible to get a working build of Electron until we've caught up and ironed out a full upgrade. We try to keep upgrades as small as possible for exactly the reason you point out, but in reality the duration an upgrade covers varies depending on the work required and other circumstances.

I hope it helps to hear I do believe you and I believe that this issue is real. It seems like a very interesting problem to me, but there is unfortunately nothing we can do right now until you or someone else can narrow the problem space down to something we can dive into. Best of luck!

Footnotes

  1. Sure, I can come up with potential long-shots like compiler/optimizer bugs and C++ const-based implementation selection differences, but I am confident they all lead to dead ends.

@clavin clavin added the blocked/need-repro Needs a test case to reproduce the bug label May 21, 2024
@Nantris
Copy link
Contributor Author

Nantris commented May 22, 2024

@clavin thank you very much for taking the time to explain all of that; it all makes good sense and I really appreciate the detailed explanation!

I wonder if there would be a benefit in sharing potentially relevant Chromium PRs (independent of having a functional repro, in case I never do manage to reproduce it.) I have been combing the Chromium changelog between the relevant versions looking for anything potentially related.

I'll continue trying to track this down and report back when I have any updates on this sneaky bug.

@electron-issue-triage electron-issue-triage bot removed the blocked/need-repro Needs a test case to reproduce the bug label May 22, 2024
@Nantris
Copy link
Contributor Author

Nantris commented May 22, 2024

It looks like the bot removed the blocked/need-repro tag for some reason.

@Nantris
Copy link
Contributor Author

Nantris commented May 23, 2024

I forgot we use enable-experimental-web-platform-features to enable Houdini Animation Worklets. It appears that this bug is related to enable-experimental-web-platform-features

I still don't have a repro, but at least I have a foot in the door now.

@Nantris
Copy link
Contributor Author

Nantris commented May 23, 2024

Indeed appears to be a Chromium bug. I guess I shouldn't get my hopes up this will be fixed any time soon.

@Nantris Nantris closed this as not planned Won't fix, can't repro, duplicate, stale May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants