456088 - Ctrl+Tab / All Tabs revision

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Dão Gottwald [:dao]]

Attachment: patch

This is a rather small update for All Tabs but a major one for Ctrl+Tab, removing the scrolling and adding pagination. Performance is improved by using the canvas element directly instead of toDataURL and <xul:image/>.

Comment 1

[Dão Gottwald [:dao]]

(In reply to comment #0)
> Performance is improved by using
> the canvas element directly instead of toDataURL and <xul:image/>.

This also fixes bug 447896.

Comment 2

[Dão Gottwald [:dao]]

Attachment: patch v2

Comment 3

[Dão Gottwald [:dao]]

Attachment: patch v3

synced with bug 436304

Comment 4

[Simon Bünzli]

A few comments WRT the latest try-server build (on WinXP):

* This is much better than what we've previously had! I was even delighted to find that clicking on the page markers worked as expected.

* The page title remaining fixed at the top of the panel makes it harder to follow both the previews _and_ their titles, as they don't remain at a fixed distance.

* Holding Ctrl and then pressing Tab and Esc doesn't dismiss the panel and leaves no obvious way to do so (less obvious are clicking on a preview or Alt+Tabbing to Minefield and then hitting Ctrl+Tab again, but technically both of these aren't "dismissing", or focusing Minefield and hitting Esc again). I've seen a Mac-specific bug to that end. Have I missed the Windows one?

* The previews aren't updated until the panel is already visible (cf. bug 436304 comment #41), neither is the title. Visual noise...

* The currently selected tab's URL is displayed in the status bar behind the panel. IMO that's unneeded distraction, as the status bar is definitively not where you're looking and the change is perceived nonetheless.

Comment 5

[Dão Gottwald [:dao]]

(In reply to comment #4)
> * Holding Ctrl and then pressing Tab and Esc doesn't dismiss the panel

This is because bug 451300 added noauthide="true", because of bug 457997.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I did a test for this feature with the given try server build on bug 436304 and have to say it looks great! Thanks Dao. But further I've also noticed following issues (at least on OS X):

* The outline of the selected tab is not blurred. I can only see this partly applied on the first line. It has holes inside and the 2nd and 3rd line misses it at all.

* Hitting Ctrl+Tab really fast shouldn't show the dialog. I think to remember we already had this topic on another bug. Probably the one which handled the initial implementation.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: broken outline

This shows the broken outline on the first line.

In the "All tabs" panel we have a focus rect around the currently selected tab. Shouldn't it be also used for Ctrl-Tab?

Comment 8

[Dão Gottwald [:dao]]

(In reply to comment #7)
> Created an attachment (id=342473) [details]
> broken outline
> 
> This shows the broken outline on the first line.

Probably worth a separate bug. Core:GFX I would guess.

> In the "All tabs" panel we have a focus rect around the currently selected tab.
> Shouldn't it be also used for Ctrl-Tab?

The "All Tabs" panel highlights both the hovered and the focused tab, thus the extra rect in order to differentiate between them.

Comment 9

[Dão Gottwald [:dao]]

Attachment: patch v3

synced with bug 436304

Comment 10

[Dão Gottwald [:dao]]

Attachment: patch v3

updated to trunk

Comment 11

[Dão Gottwald [:dao]]

Attachment: patch (ctrl-tab only)

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mconnor, could you please give a status update for this bug? The freeze is coming soon and QA needs some time to run verification tests on this feature. Even none of the patches were reviewed yet. Doing so will probably cause remaining work which has to be done asap.

Thanks!

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 345201 [details] [diff] [review]
patch (ctrl-tab only)

looks good overall.  Could use a bit more spacing and comments throughout, but code looked pretty straightforward.

still want to test this hands on, and get a ui-r from beltzner before landing this and the all tabs patch today.  tryserver builds are in progress though.

Comment 14

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/9dcff436bd1b

Comment 15

[Jennifer Morrow [:Boriss] (UX)]

I emailed a few notes, I'll put them here too:

I'm running the latest build from Bug 436304 and Bug 456088 and see a few problems.  Here's the three biggest ones:

1. Control Tab and All Tabs are still separate items.  I believe mconnor made the call that we were creating only one preview mode.  Since both now have 12 tabs, the I think the distinction between them makes even less sense now.  The design that is posed to land is one Control+Tab with a search bar and pagination and 12 previews.

2. When the preview panes appear and disappear is broken.  Right now Control+Tab sometimes disappears when you let go of the keys, and sometimes not.  All tabs stays until a selection is made.  Here is my recommendation, assuming only one Control+Tab panel:

- If Control+Tab is called via the chrome button, it is permanent until a selection is made or the user clicks away from the screen.  

- If the user calls the window with Control+Tab, it is temporary and is dismissed if the keys are released.  This is true unless the user begins typing or moves the cursor over the window, in which case the window becomes permanent unless a selection is made or the user clicks off the screen.

3. If the user presses Control+Tab and then Tabs to a preview and releases, Control+Tab should go to that window and be dismissed.  Right now, often nothing happens if the user tabs over.

Comment 16

[Sam]

So are we going to merge with Ctrl+Tab and lose All Tabs widget?
Many guys use it to survey their tabs, and they are in the same order they positioned on a tabbar strip.

Visualization of tabs brought by new Ctrl+Tab behavior is good, but it messes up the order of tabs you still have on said tab strip.

I'm [was] using keyconfig to invoke 'List All Tabs' widgets with keyboard shortcut, and for one would not like to loose this ability to have a simple flat list of tabs to observe.

Very difference of Ctrl+Tab & 'List ALL Tabs' entity suggest that we keep them separate & with distinct workings.

Yes there's 80% of the market to take, but not at the cost of embarrassing your loyal 20% home base.

Comment 17

[Jennifer Morrow [:Boriss] (UX)]

Re Comment 16 -

The difference I was referring to between Control+Tab and All Tabs is the ordering of the tabs: tab order vs. MRU.  Since both now order tabs in the same way, there's no longer a distinction between them and they are instead two panels of the same feature.  So this at least is resolved.

Re having order - bug 463569 has been resolved.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This has been landed a while back before it was removed from 1.9.1 branch. Marking as verified1.9.1.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Marcia, how can we solve the litmus flag? Which tests you want to add?