Closed Bug 1669866 Opened 4 years ago Closed 4 years ago

[Big Sur beta] Clicks on tab close buttons are sometimes ignored

Categories

(Core :: Widget: Cocoa, defect)

Unspecified
macOS
defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bradwerth, Assigned: bradwerth)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

Steps to Reproduce (intermittently):

  1. Create 5 tabs.
  2. Click amongst the tabs to select them a few times.
  3. Attempt to close each of the tabs by clicking on the tab close buttons.

Expected Results:
Tab should close.

Actual Results:
Tab doesn't close. When triggered, this bug will continue to manifest. The only way to close the tab is to choose the Close Tab menu item or use the keyboard shortcut.

Summary: [Big Sur beta] Clicks on tab close buttons are sometimes ignored. → [Big Sur beta] Clicks on tab close buttons are sometimes ignored

In my attempt to debug this, I discovered that [ToolbarWindow sendEvent] calls [NSWindow sendEvent] for most events, and when the bug is manifesting, events of type NSLeftMouseDown are no-op'ing that call. NSLeftMouseUp events continue to be processed normally, but that causes mClickCount to be zero and the event is ignored. The Apple documentation for sendEvent says that this method should not be called directly. I tried removing the implementation of [ToolbarWindow sendEvent] and that seems to fix the bug. But that takes out the special processing for modal windows introduced in very old Bug 395465. That bug does not include a test, so I can't check if this code is still needed. I suspect that it is not needed; presentation of modal dialogs has changed a lot in macOS in the past 13 years.

I'm going to upload my patch that removes this code and see if I can determine that modal processing is still correct.

Assignee: nobody → bwerth
Component: Tabbed Browser → Widget: Cocoa
Product: Firefox → Core
Blocks: 1648487

(In reply to Brad Werth [:bradwerth] from comment #1)

I'm going to upload my patch that removes this code and see if I can determine that modal processing is still correct.

The two ways I can think to test modal behavior (with this patch applied) both seem to work. I tested window.alert and it behaves as expected. I set the browser.warnOnQuit pref to true and attempted to quit the browser and that modal seemed to work correctly as well. I'm going to seek review on this patch. For me, this issue makes the browser very difficult to use -- because it also seems to drop clicks on the main part of the tab, which makes it impossible to switch tabs.

(In reply to Brad Werth [:bradwerth] from comment #1)

and when the bug is manifesting, events of type NSLeftMouseDown are no-op'ing that call.

Do you know how / why?

(In reply to Brad Werth [:bradwerth] from comment #3)

I tested window.alert and it behaves as expected.

This one is a tab-modal dialog. It is not an OS window and would not be affected by this behavior.

I set the browser.warnOnQuit pref to true and attempted to quit the browser and that modal seemed to work correctly as well.

This is a window-modal dialog (a sheet).

To get an app-modal dialog, you can use History -> Clear Recent History. Can you test your patch again with that?

Furthermore, do you know why this only breaks clicking in the middle area where the window title would be displayed (if we hadn't set it to invisible), and not to the left / right of the window title area?

(In reply to Markus Stange [:mstange] from comment #5)

To get an app-modal dialog, you can use History -> Clear Recent History. Can you test your patch again with that?

Thanks for this. With the patch, this case is not handled correctly. The user can still interact with the page behind the modal. I'll see if I can rework the patch to handle this correctly, while still getting the desired fix.

(In reply to Markus Stange [:mstange] from comment #4)

(In reply to Brad Werth [:bradwerth] from comment #1)

and when the bug is manifesting, events of type NSLeftMouseDown are no-op'ing that call.

Do you know how / why?

No. In Xcode, I can't step into the [super sendEvent] call. It just bounces right off.

(In reply to Markus Stange [:mstange] from comment #7)

Furthermore, do you know why this only breaks clicking in the middle area where the window title would be displayed (if we hadn't set it to invisible), and not to the left / right of the window title area?

I hadn't noticed that, but I can reproduce that behavior. When clicks on the tab are failing, you can move the cursor to the bottom fifth of the tab, causing it to highlight, and then clicks will register correctly (in that area that triggers the highlight). There is also a one-pixel high region at the top that will consistently select the tab, though it won't highlight it first like moving to the bottom fifth. I also discovered that this problem happens even with the patch applied.

So, my current theory is that we have a layering issue with the content we put into tabs. The background of the tab is able to receive events consistently, but the title content is not. If that area of the title content covers the tab close button, it prevents tab close from working. Probably would prevent tab mute as well. I'll see if I can figure out how our layers (sub-views?) are composed and what options we have to always get a consistent event handler stack.

It looks like the error can occur even with the patch applied, so I'm obsoleting the patch. This is more evidence that there's something wrong with the way we populate child views in the tab area. Specifically, something that changes their z-ordering and prevents events from reaching the intended views.

Attachment #9180319 - Attachment is obsolete: true

This is probably fixed in macOS 11 Beta 10 (build 20A5395g).

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WORKSFORME

(In reply to Steven Michaud [:smichaud] (Retired) from comment #11)

This is probably fixed in macOS 11 Beta 10 (build 20A5395g).

I was just pushed this build and I have not been able to replicate the bug yet.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: