"Master Password Dialog" interrupts while reordering tabs. The drag operation is interrupted unintentionally

RESOLVED DUPLICATE of bug 1538460

Status

()

defect
P1
normal
RESOLVED DUPLICATE of bug 1538460
2 months ago
a month ago

People

(Reporter: alice0775, Assigned: sfoster)

Tracking

(Regression, {regression, ux-interruption})

67 Branch
x86_64
Windows 10
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 fix-optional, firefox68 fix-optional)

Details

Reporter

Description

2 months ago

Reproducible: always

Steps to reproduce:

  1. Make sure Master Password set

  2. Logout web site(ex BMO)

  3. Open web page(ex https://bugzilla.mozilla.org/ ) in background tab

  4. Attempt to reorder tab
    Drag the background tab to the left or right

Actual Results:
Suddenly, "Master Password Dialog" pops up. And The drag operation is interrupted unintentionally.

Expected Results:
The drag operation should not interrupted.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9eab7d33fb582d01e005b4125894d35b6d8e7690&tochange=d16bf53b358583baa538e24697f85a597e4cf41c

Regressed by:
d16bf53b3585 Sam Foster — Bug 1149500 - Delay autofill until the document is visible. r=MattN

Component: Tabbed Browser → Password Manager
Product: Firefox → Toolkit
Has STR: --- → yes
Assignee

Comment 1

2 months ago

Bug 1149500 made it so we defer showing master password prompt until the tab/document becomes visible (we listen for visibilitychange)

My understanding is that while we do expect tab warming to potentially happen as we mouseover each tab in the strip, this shouldn't cause a visibilitychange which would trigger the form autofill process to begin which causes the master password prompt to show.

So, I'll investigate a little to see what is actually going on.

Assignee

Comment 2

2 months ago

Turns out this only reproduces if the tab you want to drag is one which would require your master password. The other background tabs are not impacted. So, the mousedown that starts the drag also serves to bring the tab to the foreground and trigger the master password prompt to show.

I suspect we don't want to change the mouse-down-brings-background-tab-to-foreground behavior.

Detecting a tab drag is a little tricky - we don't currently keep any state that would say "this tab is in the process of being dragged". Technically, any mousedown on the tab starts a drag, we just abort handling it if the distance moved was below some threshold. However, finding a way to wait for the drag to end or mouseup to happen before processing the _onVisibleTasksByDocument in LoginManagerContent.jsm is one potential direction to take.

Assignee

Updated

2 months ago
Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Assignee

Updated

2 months ago
Priority: -- → P1

An idea which probably won't work… does window.requestIdleCallback start an idle period while the user is dragging the tab?

From the spec

The user agent is in a better position to determine when background tasks can be run without introducing user-perceptible delays or jank in animations and input response, based on its knowledge of currently scheduled tasks, vsync deadlines, user-interaction and so on.

I'm guessing that we don't consider tab drags as a time when we're not idle especially since they're in a different process than the content but it could be worth a try.

Sam, do you have an update on this 67 regression?

Flags: needinfo?(sfoster)
Assignee

Comment 5

a month ago

(In reply to Pascal Chevrel:pascalc from comment #4)

Sam, do you have an update on this 67 regression?

:pascalc, I have a patch in progress. Most of the work is being done in bug 1538460. It may turn out we can dupe this bug to that one, but I'll need to confirm.

(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #3)

An idea which probably won't work… does window.requestIdleCallback start an idle period while the user is dragging the tab?

:MattN tried that it doesnt help unfortunately.

I'm currently trying to round out the approach outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=1538460#c24

Flags: needinfo?(sfoster)
Assignee

Comment 6

a month ago

This and bug 1538460 share the same root cause, and the fix for this regression is the same so I'm duping them.

Status: ASSIGNED → RESOLVED
Last Resolved: a month ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1538460
Assignee

Updated

a month ago
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.