625465 - Items in list view should launch description view with a single click on an add-on entry

Status: RESOLVED INVALID

(Toolkit :: Add-ons Manager, defect, P3)

Description

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Mockup: Arrow instead of "more" link adds simplicity and reduces text

tl;dr: A patch for this bug should: (1) make descriptive view one click away in list view, (2) add a mouseover state for list view entries, and (3) add an arrow graphic at the end of each list view description.

Currently, there are two ways for a user to go from list view in the add-ons manager to description view.  

1. Double clicking an entry in list view.  Double clicking is largely a desktop interaction that is rarely used on the web and should be avoided where possible.  The only current uses of double clicking within Firefox are actions meant to simulate desktop interactions (eg double clicking a bookmark in the Library to open it, as though it were a file or application).  The other problem with double clicking is that it's utterly undiscoverable.

2. Clicking the "more" link after an add-on's description.  This has been in the design since the original mockups, and is certainly more discoverable than double clicking.  However, feedback from the nightlies has consistently shown that users don't click on "more" and aren't aware that this opens description view.  I've sat down with a few users and told them to open list view and then get more information about a particular add-on.  Most say they don't know how to access additional information.  I then suggest they click on "more," and the response is usually: "oh, *that's* what that does."  Some users clicked once on an add-on entry.

Because description view is currently not easily discoverable, the design is being changed so a single click on an item in list view launches description view.

This is a move that has some benefits and drawbacks, but whose benefits I feel outweigh the drawbacks.

Benefits:

- Simplified UI.  "More" button is no longer needed.

- Easier to discover.  Clicking once on an entry was the action most users took to get more information about an add-on when asked.  This is probably because the items in list view already have some affordances of buttons in their styling - they're square, like targets, and have shading and gradients similar to buttons.  On desktop computers, a subtle mouseover coloring will reinforce further that each item is a button.  On touch devices, a single click on a large target delve in is arleady  with most UI.  

- Easier target to get to descriptive mode.  With a button the size of list view, it should be quicker and easier for users to view more of an add-on's details.

Drawbacks:

- Makes multi-select harder for future releases.  The reason that individual list view items were not specificed to be buttons in the original design is because multi-select was in the plan for Firefox 4.0.  Multi-selection would allow users to quickly take bulk actions on add-ons without finding a preference or making many individual actions.  However, multi-select is no longer planned for the add-ons manager in Firefox 4.0, and to lose out on the benefits because this may be a part of a future design seems be a net loss.  The add-ons manager is functional without multi-select, which would always be an expert and hidden feature, even if implemented.  Ultimately, we should be designing each feature optimally for how the add-ons manager is and not how it may be.

- The fact that two buttons (remove and disable) will still be present in list view makes the clickable list view item somewhat inconsistent with other Firefox widgets.  The two buttons on each entry essentially are buttons-within-buttons, which is an unusual move that is taken nowhere else in Firefox.  An added disadvantage is that missing the target for remove and disable could mean accidentally entering descriptive view.  However, I feel this drawback is worth the advantages.  The two buttons that remain are important to leave in the forefront of list view because they are the most common interactions users would take with add-ons, and most users should never need descriptive view.  To users that do use descriptive view, it's an easy single click to enter.  If descriptive view is accidentally access, a single click on the back buttons restores the previous screen.  I don't think users will find this somewhat unusual widget hard to understand and use.

If one click on a list view item were the only way to enter descriptive view, it's possible it would still not be discoverable enough.  Even though the list view items have affordances of clickable buttons, the fact that there are buttons-within-buttons may make users hesitate to click on the list view entry itself.  To this end, placing an arrow graphic at the end of the description of an add-on would show that there is more information a click away.  If users only use this arrow "button" to enter descriptive view, all they are missing is an easier target.  If users know that one click enters descriptive view, a single arrow graphic should not clutter the list view UI too much.

Comment 1

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

That's kinda hard switch compared what we have right now. The entries in the different categories still feel like a listbox items. I believe that at least at the beginning users will be irritated with this behavior. Especially that a complete new page appears immediately. Something like sliding the content of the listbox out of the visible area and slide in the detail page would IMO help users a lot here to understand what's happening.

Otherwise I agree with the broken discovery of the details view, and that we should improve it.

Comment 2

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

I assume you also meant blocking2.0 and not Fennec.

Comment 3

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #2)
> I assume you also meant blocking2.0 and not Fennec.

Yep.

(In reply to comment #1)
> That's kinda hard switch compared what we have right now. The entries in the
> different categories still feel like a listbox items. I believe that at least
> at the beginning users will be irritated with this behavior. Especially that a
> complete new page appears immediately. Something like sliding the content of
> the listbox out of the visible area and slide in the detail page would IMO help
> users a lot here to understand what's happening.
> 
> Otherwise I agree with the broken discovery of the details view, and that we
> should improve it.
Some sliding animation may be possible in the future.  And I agree that this behavior may surprise people at first.  However, I think it's far worse that right now, no one knows how to get more information about an add-on.

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: WiP v1

Makes item select-on-hover, and enter detail view with a single click. Needed a bunch of test fixes. 

It *doesn't* currently test that hover selects items - sending mousemove events via EventUtils.synthesizeMouse() isn't working for me, and I have no idea why. Any hints would be greatly appreciated!

I'm blocked on images from Boriss. Specifically, an image for the button that's to replace the "More" link - for Windows and OSX (Linux should have a stock icon), for normal, hover, and maybe active states. Also mockups (or at least a description) showing what a hovered item looks like (since there was talk of it being slightly different). Note: if this takes more than a day or two more, I suggest this gets landed without it and have that fixed in a followup bug. This sort of change is something that needs beta coverage, IMO.

Comment 5

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

(In reply to comment #4)
> It *doesn't* currently test that hover selects items - sending mousemove events
> via EventUtils.synthesizeMouse() isn't working for me, and I have no idea why.
> Any hints would be greatly appreciated!

Do you have a snippet so we can get more insight in which specific code you have used for the test?

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: WiP test (not working)

Here's the unfinished test I was trying to get working (I meant to attach it earlier). The hover-on-select code is basically the same as what the autocomplete binding uses (with some cleaning up and handling scrolling)... but I couldn't find any test for that.

Comment 7

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

Comment on attachment 506712 [details]
WiP test (not working)

>function get_item_center(aItem) {
>  var list = gManagerWindow.document.getElementById("addon-list");
>  return {
>    x: (aItem.boxObject.x - list.boxObject.x) + (aItem.boxObject.width / 2),
>    y: (aItem.boxObject.y - list.boxObject.y) + (aItem.boxObject.height / 2)
>  };

That's not needed when you use synthesizeMouseAtCenter:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/EventUtils.js#233

But shouldn't we better use the top left corner, i.e. (10, 10)? Not sure if for smaller windows the buttons are already present in the middle of the list entry.

>  EventUtils.synthesizeMouse(list, itemCenter.x, itemCenter.y, {type: "mousemove"}, gManagerWindow);

Can you check if the list entry receives the correct event? synthesizeMouseExpectEvent would be a good helper to test that.

Comment 8

[Dave Townsend [:mossop]]

(In reply to comment #4)
> Created attachment 506634 [details] [diff] [review]
> WiP v1
> 
> Makes item select-on-hover, and enter detail view with a single click. Needed a
> bunch of test fixes. 

Do we actually need select-on-hover? All the bug requests is a mouseover state

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #8)
> Do we actually need select-on-hover? All the bug requests is a mouseover state

Yes - to keep keyboard navigation in sync and working sanely. It feels very wrong (and confusing) without that.

Comment 10

[Dave Townsend [:mossop]]

Comment on attachment 506634 [details] [diff] [review]
WiP v1

In general looks ok but I'm suspicious of the selection logic. Some comments explaining what it is intending to do would help. As far as I can tell as soon as the mouse enters any richlistitem it will get selected (because a mouseout will have fired making mLastMoveTime null so Date.now() is large enough to trigger the change). So I'm not sure what the rest of the timing stuff in the mousemove handler is meant to be doing.

Comment 11

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Mockup: Color and texture on normal list view, on mouseover, and on click

The attached is a guide for the colors and textures of list view for Windows 7 and OSX.

When the user mouses over an item in list view, a color change should suggest that the add-ons are clickable targets. 
When the user clicks, the add-on’s entry should recess slightly, producing a shadow consistent with the texture of the manager.

Windows 7 Normal:

Enabled add-ons: background varies with shine and gradient
Disabled add-ons: #e1e5eb solid background
Blocked add-ons: #f1d5da solid background

Windows 7 Mouseover:

Enabled add-ons: #f3f7fb solid background
Disabled add-ons: #f3f7fb solid background, faded text and icon do not change
Blocked add-ons: #efe9ec solid background,  faded text and icon do not change

Windows 7 On Click:

Enabled add-ons: #edf3f8 solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide
Disabled add-ons: #edf3f8 solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide
Blocked add-ons: #f1e3e7 solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide

OSX Normal:

Enabled add-ons: about #c9cfd8, varies slightly due to gradient
Disabled add-ons: #c8cdd3 solid background
Blocked add-ons: #d9bcc2 solid background

OSX Mouseover:

Enabled add-ons: #e3e6eb solid background
Disabled add-ons: #e3e6eb solid background (faded text and icon never change)
Blocked add-ons: #ddcad0 solid background (faded text and icon never change)

OSX On Click: 

Enabled add-ons: #d6dbe2 solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide
Disabled add-ons: #d6dbe2 solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide
Blocked add-ons: #dac2cb solid background, black shadow, 15% opacity, 120° angle, 1px distance, 4px wide

Comment 12

[Jennifer Morrow [:Boriss] (UX)]

Photoshop file for the attached guide can be found at https://people.mozilla.com/~jboriss/bugs/bug625465/

Comment 13

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Mockup: Guide to arrow icon that links to detailed view

I'm attaching a guide to the arrow button shown in the mockups and the graphic files themselves.

Comment 14

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Normal arrow icon

Comment 15

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Moused over arrow icon

Comment 16

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Clicked arrow icon

Comment 17

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #9)
> (In reply to comment #8)
> > Do we actually need select-on-hover? All the bug requests is a mouseover state
> 
> Yes - to keep keyboard navigation in sync and working sanely. It feels very
> wrong (and confusing) without that.

As long as I'm spamming everyone mercilessly - Blair, are there any other graphics or files you need here?

Comment 18

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Normal arrow icon

Comment 19

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Clicked arrow icon

Comment 20

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Graphic file: Clicked arrow icon

Comment 21

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #10)
> In general looks ok but I'm suspicious of the selection logic. Some comments
> explaining what it is intending to do would help. As far as I can tell as soon
> as the mouse enters any richlistitem it will get selected (because a mouseout
> will have fired making mLastMoveTime null so Date.now() is large enough to
> trigger the change). So I'm not sure what the rest of the timing stuff in the
> mousemove handler is meant to be doing.

I removed the mouseout handler - I can't remember why I added that, and it seemed questionable/wrong. But essentially, the timing stuff ensures we don't kill performance, by making sure at least 30ms has passed since the last update. That code is an adaption of what the autocomplete listbox binding does (with stuff added to make scrolling update the selected item too).

Comment 22

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Updated with comments, and the new styles for selected/pressed items, and the "more" button.

Does *not* have a test, yet.

Comment 23

[Jennifer Morrow [:Boriss] (UX)]

Attachment: Screenshot: Shadow on Patch v1 vs mockup

(In reply to comment #22)
> Created attachment 511517 [details] [diff] [review]
> Patch v1
> 
> Updated with comments, and the new styles for selected/pressed items, and the
> "more" button.
> 
> Does *not* have a test, yet.

This is looking great!  One slight correction on OSX: the on-click shadow is just a bit too dark compared to the mockups (see attached) - could you decrease the opacity on that a bit?

Comment 24

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #23)
> This is looking great!  One slight correction on OSX: the on-click shadow is
> just a bit too dark compared to the mockups (see attached) - could you decrease
> the opacity on that a bit?

Done - matches much better now (hadn't noticed until that screenshot). Gonna try to finish the test off today (assuming nothing else gets in the way again).

Comment 25

[Dave Townsend [:mossop]]

Comment on attachment 511517 [details] [diff] [review]
Patch v1

Looks ok though Boriss commented on an issue that needs fixing. It looks like you are testing that clicking on the list does go to the detail view, maybe making the selection test test using the mouseover event is all that is needed otherwise here?

Comment 26

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2

A test! Also made a small change to the way the timing is handled, since testing uncovered that the first mousemove event was being ignored. And I've reduced the opacity of the inset shadow on OSX by 5% (sounds like a tiny amount, but 10% made it too light).

Comment 27

[Mike Beltzner [:beltzner, not reading bugmail]]

Should this get reviewed, it has implicit a=beltzner

Comment 28

[Dave Townsend [:mossop]]

Comment on attachment 512698 [details] [diff] [review]
Patch v2

Looks good however please don't include the string removal in the initial landing. Since this is a softblocker if we find some regressions from this then it'll have to bounce and I don't want that to turn into an l10n problem. We can do the string removal after a few days when it looks clear.

Comment 29

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.01

Patch without the string removal, ready for landing.

Comment 30

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch - string removal only, DON'T LAND

Separate string removal patch.

Comment 31

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/6b3353ea6228

not closing bug since there is still the string removal patch... May I suggest to move it to a separate dedicated bug so this can be closed?

Comment 32

[Marco Bonardo [:mak]]

backed out due to permaorange
http://hg.mozilla.org/mozilla-central/rev/fbb6c03b8cbc
http://hg.mozilla.org/mozilla-central/rev/6e3e92057d43

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1297949342.1297950589.25787.gz

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the disable button - Got [object HTMLInputElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser-window/browser_list.js | Focus should have moved to the list - Got [object HTMLInputElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the disable button - Got [object HTMLInputElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the remove button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the more button - Got [object XULElement], expected [object XULElement]
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_list.js | Focus should have moved to the list - Got [object XULElement], expected [object XULElement]

Comment 33

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.02 - ready for checkin

*sigh* Test bitrot. The new focus additions to browser_list.js was clicking on an item, expected it to only gain focus.focus it.

Comment 34

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In case someone gets to it before I do)

Comment 35

[al_9x]

what will happen when you back out of the description view, will you preserve the scroll position in the list?

Comment 36

[custom.firefox.lady [:tawn]]

(In reply to comment #0)
>  If descriptive view is accidentally access, a single
> click on the back buttons restores the previous screen.  

Will it really, or will it return us to the *top* of the extension list, making us scroll all the way down again?

Comment 37

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #35)
> what will happen when you back out of the description view, will you preserve
> the scroll position in the list?

That's bug 586956.

Comment 38

[(no longer active)]

Landed <http://hg.mozilla.org/mozilla-central/rev/8c2aa133200a> and backed out <http://hg.mozilla.org/mozilla-central/rev/f523472a47e6> because of test failures:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1298062783.1298066033.23047.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1298062587.1298065895.22796.gz
etc.

Note that this took bug 631827 down with it.

Comment 39

[Bill Gianopoulos [:WG9s]]

This seems to fail mochitest-other on debug builds on all platforms and OPT builds on Windows.

Comment 40

[Bill Gianopoulos [:WG9s]]

Test failure is:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/mozapps/extensions/test/browser/browser_bug625465.js | Correct item should be selected - Got 1, expected 3

Comment 41

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.03

Fixed test - passes on slower debug builds now.

However, I'm gonna ask for re-approval since it missed the final beta (in comment 4 I said I wanted beta feedback). I think the code is relatively safe and is all tested, but the UI change is going to be a surprise.

Comment 42

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 514461 [details] [diff] [review]
Patch v2.03

While I agree that this is a behavioural change, I think it's the one that will be expected and come across like a bugfix. Thanks for checking, though!

a=beltzner

Comment 43

[(no longer active)]

This needs a new patch, as the current patch doesn't apply cleanly and the merging is not trivial.

Comment 44

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v2.04

*sigh* Bitrot. Including with one of my own patches.

Comment 45

[Dão Gottwald [:dao]]

Comment on attachment 515038 [details] [diff] [review]
Patch v2.04

>--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
>+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css

>+.details:-moz-focusring > .button-box {
>+  border-color: transparent;
> }

Other than breaking keyboard focus feedback, what's this supposed to do?

Comment 46

[Dão Gottwald [:dao]]

(In reply to comment #45)
> Comment on attachment 515038 [details] [diff] [review]
> Patch v2.04
> 
> >--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> >+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> 
> >+.details:-moz-focusring > .button-box {
> >+  border-color: transparent;
> > }
> 
> Other than breaking keyboard focus feedback, what's this supposed to do?

Since the richlistitem is focusable and Enter activates it, I don't think you want this button to be focusable at all.

While playing with the patch, I also noticed that scrolling through the list with the arrow keys repeatedly focuses the item under the mouse cursor when it rests somewhere on the list. :\

Comment 47

[Dão Gottwald [:dao]]

(In reply to comment #0)
> Benefits:
> 
> - Simplified UI.  "More" button is no longer needed.

This doesn't work on Linux, since the new icon seems visually more complex than the "more" link. Can we entirely remove this thing and just use the hand pointer over the entire item (except for the inner buttons)?

Comment 48

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #47)
> (In reply to comment #0)
> > Benefits:
> > 
> > - Simplified UI.  "More" button is no longer needed.
> 
> This doesn't work on Linux, since the new icon seems visually more complex than
> the "more" link. Can we entirely remove this thing and just use the hand
> pointer over the entire item (except for the inner buttons)?

Is there a Linux icon equivalent we could substitute in?  Is the actual problem the button color?  Maybe I could add a more transparent version for Linux.

Comment 49

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #46)
> (In reply to comment #45)
> > Comment on attachment 515038 [details] [diff] [review]
> > Patch v2.04
> > 
> > >--- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> > >+++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css
> > 
> > >+.details:-moz-focusring > .button-box {
> > >+  border-color: transparent;
> > > }
> > 
> > Other than breaking keyboard focus feedback, what's this supposed to do?
> 
> Since the richlistitem is focusable and Enter activates it, I don't think you
> want this button to be focusable at all.
> 
> While playing with the patch, I also noticed that scrolling through the list
> with the arrow keys repeatedly focuses the item under the mouse cursor when it
> rests somewhere on the list. :\

While it's not a big deal, I think it's be preferable to make the more icon focusable. The reason is that it's essentially there to give people a visual cue that there's more information, since not all users will know that the entire list view item is clickable. Not making the item focusable to such users will reintroduce the problem. To them it could seem that focus is simply broken, and thus there's no way to move to detailed view while just using the cursor.

Comment 50

[Dão Gottwald [:dao]]

(In reply to comment #48)
> > > Benefits:
> > > 
> > > - Simplified UI.  "More" button is no longer needed.
> > 
> > This doesn't work on Linux, since the new icon seems visually more complex than
> > the "more" link. Can we entirely remove this thing and just use the hand
> > pointer over the entire item (except for the inner buttons)?
> 
> Is there a Linux icon equivalent we could substitute in?

I don't think so...

> Is the actual problem the button color?

I got an orange icon, so yes, the color is part of the problem.

> Maybe I could add a more transparent version for Linux.

Why does the icon exist at all? Removing it should be a simplicity win across the board.

Comment 51

[Dão Gottwald [:dao]]

(In reply to comment #49)
> While it's not a big deal, I think it's be preferable to make the more icon
> focusable. The reason is that it's essentially there to give people a visual
> cue that there's more information, since not all users will know that the
> entire list view item is clickable.

Shouldn't the mouse cursor expose the clickability?

> Not making the item focusable to such users
> will reintroduce the problem. To them it could seem that focus is simply
> broken, and thus there's no way to move to detailed view while just using the
> cursor.

Are you referring to keyboard users now? For them I think a focus ring on the whole item is needed, as specified in the default richlistitem styling: http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/richlistbox.css#62

Comment 52

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #50)
> (In reply to comment #48)
> Why does the icon exist at all? Removing it should be a simplicity win across
> the board.

1. It's a more minimal alternative to the more link we were using before
2. It's more consistent than the more link, as blue underlined links are being used for external URLs and not expanded information in the manager
3. It gives a visual cue to show the invisible information that clicking will provide more information

> Shouldn't the mouse cursor expose the clickability?

The changing style and cursor should suggest clickability, yes.  That's the  reason for showing a mouseover state.

> Are you referring to keyboard users now? For them I think a focus ring on the
> whole item is needed, as specified in the default richlistitem styling:
> http://mxr.mozilla.org/mozilla-central/source/toolkit/themes/winstripe/global/richlistbox.css#62

There should definitely be a focus ring on the whole item. I was suggesting that we might want to have one on just the arrow button as well. But again, that's not a big deal.

Blair, would a semi-transparent version of the more button solve the problem on Linux that Dao is referring to?

Comment 53

[Dão Gottwald [:dao]]

(In reply to comment #52)
> > Shouldn't the mouse cursor expose the clickability?
> 
> The changing style and cursor should suggest clickability, yes.  That's the 
> reason for showing a mouseover state.

The patch adds the mouseover state only for the icon. I'm asking why it's not there for the whole clickable list item.

Comment 54

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #53)
> (In reply to comment #52)
> > > Shouldn't the mouse cursor expose the clickability?
> > 
> > The changing style and cursor should suggest clickability, yes.  That's the 
> > reason for showing a mouseover state.
> 
> The patch adds the mouseover state only for the icon. I'm asking why it's not
> there for the whole clickable list item.

Oh! I see what you're saying!  Sorry that I was unclear - the entire list view item *should* have a mouseover state cursor, because the whole item is a button.

Comment 55

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 514461 [details] [diff] [review]
Patch v2.03

a-; sorry, we're closing up for RC, this has languished as approved for too long, now too late.

Comment 56

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Fair enough - sadly, this ended up having a bunch of different holdups (and playing with the latest changes, it still doesn't feel right).

Comment 57

[Mike Beltzner [:beltzner, not reading bugmail]]

** PRODUCT DRIVERS PLEASE NOTE **

This bug is one of 7 automatically changed from blocking2.0:final+ to blocking2.0:.x during the endgame of Firefox 4 for the following reasons:

 - it was marked as a soft blocking issue without a requirement for beta coverage

Comment 58

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #56)
> Fair enough - sadly, this ended up having a bunch of different holdups (and
> playing with the latest changes, it still doesn't feel right).

Sadness. Blair, are you willing poke this again after 4?

Comment 59

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(Bah, forgot to hit submit on Friday.)

Yes, want to get this finished still. 

Having the mouse cursor turn into a pointer when hovering over items, but not the buttons in those items, feels awkward. Will get a Try build for you to play with.

Comment 60

[Jennifer Morrow [:Boriss] (UX)]

(In reply to comment #59)
> (Bah, forgot to hit submit on Friday.)
> 
> Yes, want to get this finished still. 
> 
> Having the mouse cursor turn into a pointer when hovering over items, but not
> the buttons in those items, feels awkward. Will get a Try build for you to play
> with.

The cursor shouldn't change when over a button - the user's only moving from a button to a more different button.  The button changing on hover should be enough to point out the distinction.

Comment 61

[(no longer active)]

Blair, can you please provide a patch which can be landed on cedar (or alternatively, land it yourself over there?)

Thanks!

Comment 62

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v3

Woo! Blast from the past!

This is an unbitrotten version of the previous version of the patch, and addresses all remaining issues bought up in previous comments (such as comment 46 and comment 54).

Been tinkering with this about once every 6 months. The test has been the thorn in my side (specifically on Linux) - managed to finally get it working reliably (4 successful Try runs) this week, after converting over to the new DOM3 WheelEvent.

For review, changes that aren't just fixing bitrot should be limited to:
* event handling code in extensions.xml
* final change in content/extensions.css
* browser_list_mouse.js (formally browser_bug625465.js)

Comment 63

[Dave Townsend [:mossop]]

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #60)
> (In reply to comment #59)
> > (Bah, forgot to hit submit on Friday.)
> > 
> > Yes, want to get this finished still. 
> > 
> > Having the mouse cursor turn into a pointer when hovering over items, but not
> > the buttons in those items, feels awkward. Will get a Try build for you to play
> > with.
> 
> The cursor shouldn't change when over a button - the user's only moving from
> a button to a more different button.  The button changing on hover should be
> enough to point out the distinction.

So we probably need to fix bug 646713 too then.

Comment 64

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Dave Townsend (:Mossop) from comment #63)
> So we probably need to fix bug 646713 too then.

Ugh, yea. Off to do more yak shaving, I guess....

Comment 65

[Dave Townsend [:mossop]]

Comment on attachment 663687 [details] [diff] [review]
Patch v3

Review of attachment 663687 [details] [diff] [review]:
-----------------------------------------------------------------

Probably only simple stuff here but I'd really like for bug 646713 to be fixed before this lands.

::: toolkit/mozapps/extensions/content/extensions.css
@@ +21,5 @@
>    -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#sorters");
>  }
>  
> +.list {
> +  -moz-binding: url("chrome://mozapps/content/extensions/extensions.xml#addons-richlistbox");  

trailing whitespace

::: toolkit/mozapps/extensions/content/extensions.xml
@@ +508,5 @@
> +            item = item == this ? null : item.parentNode;
> +  
> +          if (!item)
> +            return;
> +  

trailing whitespace on a bunch of the above lines

@@ +526,5 @@
> +
> +        var now = Date.now();
> +
> +        if ((!this.mLastMoveTime) ||
> +            (now - this.mLastMoveTime > this.mMouseMoveThreshold)) {

So this means it's technically possible to end up with the mouse over a different element to that selected?

How about instead using DeferredTask to update the selection 30ms after the last mousemove/scroll?
Or would using mouseover/mouseout events be better?

@@ +537,5 @@
> +            return;
> +
> +          var index = this.getIndexOfItem(item);
> +          if (index != this.selectedIndex)
> +            this.selectedIndex = index;

A bunch of the above is repeated in _handleMouseScroll. It's not a lot but I think it still might be worth sharing the code to make sure they both get updated when necessary.

@@ +552,5 @@
> +        if (this.mMouseScrollEvents == 0)
> +          return;
> +        this.mMouseScrollEvents--;
> +        this._handleMouseScroll();
> +      ]]></handler>

I don't really get what's going on with these two handlers, can you add some comments?

@@ +1671,5 @@
>        <handler event="click" button="0"><![CDATA[
> +       if (event.originalTarget.localName != "button" &&
> +           event.originalTarget.localName != "checkbox" &&
> +           !event.originalTarget.classList.contains("text-link")) {
> +          this.showInDetailView();

I'm a little worried that this blocklist might break extension added content. Is there a short whitelist we can use instead, say label, description, image and richlistitem?

Comment 66

[Mark Striemer [:mstriemer]]

This no longer applies to the HTML version, it opens with a single click.