Closed Bug 1017904 Opened 7 years ago Closed 2 years ago

allow selecting messages via selection boxes instead of classic selection

Categories

(Thunderbird :: Folder and Message Lists, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 70.0

People

(Reporter: walgie, Assigned: alta88)

References

(Blocks 1 open bug)

Details

Attachments

(8 files, 11 obsolete files)

732 bytes, image/svg+xml
Details
25.96 KB, patch
aceman
: review+
Paenglab
: ui-review+
alta88
: ui-review+
Details | Diff | Splinter Review
2.06 KB, image/png
Details
81.63 KB, image/png
Details
712 bytes, image/svg+xml
Details
10.03 KB, patch
alta88
: review+
Details | Diff | Splinter Review
8.16 KB, patch
alta88
: review+
Details | Diff | Splinter Review
1.25 KB, patch
darktrojan
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (Windows NT 5.1; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140506152807

Steps to reproduce:

This is a request for a future feature.


Actual results:

This is a request to place a selection box to the left of each message.


Expected results:

When a single message is selected or several messages are selected by the selection box, right clicking should bring up a selection menu that contains delete the message/s plus other selections.
This could actually be interesting.
So you propose to add a selection box before each message where you click to put a checkmark to select the message. So the current set of selected messages woulc be comprised of the messages having the checkmark, not those that are selected by the current highlight system. The checkmarks would persist even if you loose the selection e.g. by mis-clicking. This would allow complex selection without the risk of loosing it easily. This could also be useful for people with disabilities/elderly or with buggy hardware.
This could be done as an optional column in the message list pane. If the column is shown, the checkmarks are respected. If not, normal selection rules apply. This would hopefully need just some small change to functions like nsMsgDBView::GetSelectedIndices (but not sure if they would be happy about peeking into UI elements).

Also, there is an existing implementation of this: Lotus Notes does it.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: x86 → All
Summary: inbox message selection boxes → allow selecting messages via selection boxes instead of classic selection
Version: 11 → Trunk
Blocks: 1019652

Many list UIs have such a column, even webmail interfaces have one as well. Most users can't find ctrl-click or shift-click or even ctrl-a and would prefer mousing to manage the threadpane list. There is an excellent extension Checkbox Column (which I had a small bit of input on) that does this perfectly. I will implement this feature based on the functionality in that extension, if UX preapproved. aleca? paenglab?

Flags: needinfo?(richard.marti)
Flags: needinfo?(alessandro)

I think this makes sense, at least for touch based devices. Maybe not shown by default.

Flags: needinfo?(richard.marti)

I agree with Richard.
It could be listed in a dedicated column users can show/hide if they want.

Flags: needinfo?(alessandro)
Attached patch selectCol.patch (obsolete) — Splinter Review

Features:

  • Add optional Select Messages column to threadpane
  • Multiselect/deselect messages with one click
  • Double click on thread root or grouped header row toggle selects/deselects all child messages
  • Toggle select all on column header click
  • Three way column header icon: indicates selection of some, all, no messages
  • Deselection of all messages prevents unwanted selection on message moves

Notes:
Grouped by header css has been consolidated into shared; the substantive change is margin-bottom.
Appearance has been tested on linux but hopefully is generic enough to work on win/osx as well,
Tested to ensure deselection does not affect message in tab/window operations.

The extreme tester will notice blank or nonmatching virtual saved search folder messages in a tab when switching tabs, this is an existing bug (Bug 1476880).

Assignee: nobody → alta88
Attachment #9084016 - Flags: ui-review?(richard.marti)
Attachment #9084016 - Flags: review?(alessandro)
Attached patch selectCol.patch (obsolete) — Splinter Review
Attachment #9084016 - Attachment is obsolete: true
Attachment #9084016 - Flags: ui-review?(richard.marti)
Attachment #9084016 - Flags: review?(alessandro)
Attachment #9084065 - Flags: ui-review?(richard.marti)
Attachment #9084065 - Flags: review?(alessandro)
Comment on attachment 9084065 [details] [diff] [review]
selectCol.patch

This works only on Linux on Mac and Windows the checkboxes in the tree are only some pixels wide. Instead of using a margin on treechildren::-moz-tree-image(selectCol), you could set a min-width on the .selectColumnHeader and padding on the treechildren::-moz-tree-image(selectCol).

The colours are hard coded and don't work with the dark theme.

I think, you shouldn't use the border around the checkmarks on the treeitems, only on the header. There are too much circles in the tree.
Attachment #9084065 - Flags: ui-review?(richard.marti) → ui-review-
Attached image selectCol_win.png (obsolete) —

How it looks on Windows. Mac is similar.

Attached patch selectCol.patch (obsolete) — Splinter Review

Thanks for the tips. It turns out win needs its own margins (padding doesn't work) and for some reason needs important. This version looks ok on both win7 and linux, and it's been changed to work with both default and dark themes without anything special referring to a dark theme.

I've not ever seen list item selection columns without any visual, checkbox(circle) etc, for unchecked state. The same paradigm is used for all cycler columns (flags, junk, read). Maybe it should be a square (circle is less boring imo; just remove border-radius) or some other indicator. Maybe you have some other visual ideas.

The only other thing would be to have a :hover 'box' appear, but that pseudo selector doesn't work for tree cell or image or column. It would mean mouseover listeners and calculating the cell and setting a property (I do it in an extension). Rather not do it here. The nsITree is very hard to work with css wise..

Attachment #9084065 - Attachment is obsolete: true
Attachment #9084065 - Flags: review?(alessandro)
Attachment #9084436 - Flags: ui-review?(richard.marti)
Attachment #9084436 - Flags: review?(alessandro)
Comment on attachment 9084436 [details] [diff] [review]
selectCol.patch

The colours are good now.  You use a lot of !important on the margins, is this needed?

On Windows the treeitem checkmark is right aligned but should be centred. The circles are too big and I think they should be square (with a small radius of around 2-3px).
You can also ask Alessandro what he thinks about the checkmarks should look.
Attachment #9084436 - Flags: ui-review?(richard.marti)

I'm pretty sure you'll need to adjust some tests, at least this one: mail/test/mozmill/folder-display/test-columns.js. Let's not find out when it lands ;-)

Comment on attachment 9084436 [details] [diff] [review]
selectCol.patch

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

I like the idea.
I have noticed these problems:
1. the checkbox in the column header is too big end expands the whole tree header row vertically (on Linux).
2. the checkboxes are actually round (circular), and containing check marks. That is strange as all other checkboxes are square.
3. when you have a few messages selected (and checkboxes ticked) if you click a new message outside the checkbox (and without e.g. Ctrl) all the selections are lost. Is that intentional?

::: mail/base/content/threadPane.js
@@ +187,5 @@
> +
> +  // There is no longer any selection, clean up for correct state of things.
> +  if (selection.count == 0) {
> +    gFolderDisplay.displayedFolder.lastMessageLoaded = nsMsgKey_None;
> +    gFolderDisplay._mostRecentSelectionCounts[1] = 0

Missing semicolon?
Also what is in the _mostRecentSelectionCounts[0] now?
Attachment #9084436 - Flags: feedback+
Comment on attachment 9084436 [details] [diff] [review]
selectCol.patch

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

This is a good start and I like that it can be controlled via dedicated column.

I'd like to suggest some changes for both UI and UX.

- UI changes
1. Let's use native checkbox elements in the message rows. This will guarantee the proper rendering of these elements and keep the visual consistency with how TB currently looks.
2. Use a native checkbox element for the column header as well and keep it as small as the header titles to not accidentally increase the column header height.
3. We should consider adding a 3rd state to the checkbox in the column header to cover the "Some messages selected" state. Something like:
 - Empty checkbox: No message selected
 - Checked checkbox: All messages selected
 - Checkbox with an inset filled square: Some messages selected

- UX changes
1. If a message is selected with a regular click on the row, the checkbox shouldn't be checked.
2. If I select multiple messages via checkboxes, and then click on a message row, my previously selected messages shouldn't be unselected. 

Great work!
Attachment #9084436 - Flags: review?(alessandro)
Attachment #9084436 - Flags: review-
Attachment #9084436 - Flags: feedback+

(In reply to Jorg K (GMT+2) from comment #11)

I'm pretty sure you'll need to adjust some tests, at least this one: mail/test/mozmill/folder-display/test-columns.js. Let's not find out when it lands ;-)

I haven't forgotten tests ;)

(In reply to :aceman from comment #12)

Comment on attachment 9084436 [details] [diff] [review]
selectCol.patch

Review of attachment 9084436 [details] [diff] [review]:

I like the idea.
I have noticed these problems:

Ok I get that this 'modernization' attempt on this ancient widget is a bridge too far..

::: mail/base/content/threadPane.js
@@ +187,5 @@

  • // There is no longer any selection, clean up for correct state of things.
  • if (selection.count == 0) {
  • gFolderDisplay.displayedFolder.lastMessageLoaded = nsMsgKey_None;
  • gFolderDisplay._mostRecentSelectionCounts[1] = 0

Missing semicolon?
Also what is in the _mostRecentSelectionCounts[0] now?

It is 0.

(In reply to Alessandro Castellani (:aleca) from comment #13)

Comment on attachment 9084436 [details] [diff] [review]
selectCol.patch

Review of attachment 9084436 [details] [diff] [review]:

This is a good start and I like that it can be controlled via dedicated
column.

I'd like to suggest some changes for both UI and UX.

I don't think people realize how inflexible, specific, and supporting limited css these nsITreeView widgets are.

  • UI changes
  1. Let's use native checkbox elements in the message rows. This will
    guarantee the proper rendering of these elements and keep the visual
    consistency with how TB currently looks.

Not possible, true native os speaking-wise. The closest analog is the Offline->Download/sync folder tree selection column. For linux/win, notchecked.gif is used and for osx, dot.svg is used. Just setting "checkbox" on the column doesn't mean you don't have to provide your own image. In case one might think |-moz-appearance: checkbox[-container]| works, it doesn't.

The next patch will use those images for unchecked, as currently done in Offline.

  1. Use a native checkbox element for the column header as well and keep it
    as small as the header titles to not accidentally increase the column header
    height.
  2. We should consider adding a 3rd state to the checkbox in the column
    header to cover the "Some messages selected" state. Something like:
  • Empty checkbox: No message selected
  • Checked checkbox: All messages selected
  • Checkbox with an inset filled square: Some messages selected

It already does tristate ;)

  • UX changes
  1. If a message is selected with a regular click on the row, the checkbox
    shouldn't be checked.

Why? It can't both be selected and not selected. Selection in the tree means very specific things for any subsequent action made on those rows.

  1. If I select multiple messages via checkboxes, and then click on a message
    row, my previously selected messages shouldn't be unselected.

As above, plus that would change 20 year old expected behavior when multiselecting then single clicking to clear all but the new selection. If such a radical change were to be made, then the selectCol would have to be not only not optional, but mandatory, and the sole way of selecting rows.

The point of the column is to easily and discoverably allow single handed multiselection and not change any existing paradigms.

Great work!

... existing paradigms, other than the unwanted autoselection after explicit deselection bug.

Attached patch selectCol.patch (obsolete) — Splinter Review
Attachment #9084436 - Attachment is obsolete: true
Attachment #9084918 - Flags: ui-review?(richard.marti)
Attachment #9084918 - Flags: ui-review?(alessandro)
Attachment #9084918 - Flags: review?(acelists)
Attached patch selectColTests.patch (obsolete) — Splinter Review
Attachment #9084141 - Attachment is obsolete: true
Attachment #9084918 - Attachment is obsolete: true
Attachment #9084918 - Flags: ui-review?(richard.marti)
Attachment #9084918 - Flags: ui-review?(alessandro)
Attachment #9084918 - Flags: review?(acelists)
Attachment #9084920 - Flags: ui-review?(richard.marti)
Attachment #9084920 - Flags: ui-review?(alessandro)
Attachment #9084920 - Flags: review?(acelists)

paenglab, the dark theme alters sizes incorrectly - on linux, the tree columns row height is 4px less, and on win it's 1 px less; this is a bug.

Attached patch selectCol.patch (obsolete) — Splinter Review
Attachment #9084920 - Attachment is obsolete: true
Attachment #9084920 - Flags: ui-review?(richard.marti)
Attachment #9084920 - Flags: ui-review?(alessandro)
Attachment #9084920 - Flags: review?(acelists)
Attachment #9084964 - Flags: ui-review?(richard.marti)
Attachment #9084964 - Flags: ui-review?(alessandro)
Attachment #9084964 - Flags: review?(acelists)

Notes: there's a typo for linux column padding, so that's fixed already. Please check osx - I used dot.svg should it be dot.png or perhaps something else? If something else, please provide an image that's suitable if it doesn't exist already.

Comment on attachment 9084964 [details] [diff] [review]
selectCol.patch

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

(In reply to alta88 from comment #22)
> Notes: there's a typo for linux column padding, so that's fixed already. 
A padding-left/right: 3px; would work too to centre the icon. 
> Please check osx - I used dot.svg should it be dot.png or perhaps something else? If something else, please provide an image that's suitable if it doesn't exist already.
I propose to move the check.gif and notchecked.gif to the shared folder and let it load on all platforms. Then you can use them on Mac too.

::: mail/themes/windows/mail/messageIcons.css
@@ +18,5 @@
> +  fill: transparent;
> +}
> +
> +.selectColumnHeader.someselected > .treecol-icon {
> +  background-color: var(--lwt-text-color);

With the dark theme this makes a white checkmark on a white background. What about to set #808080 which makes it usable for black and white checkmarks?

@@ +27,5 @@
> +  fill: currentColor;
> +}
> +
> +treechildren::-moz-tree-image(selectCol) {
> +  margin-inline-start: 6px;

This makes the checkbox right aligned.
With margin-left/right: 3px; it would be centred.
Attachment #9084964 - Flags: ui-review?(richard.marti) → feedback+

(In reply to alta88 from comment #20)

paenglab, the dark theme alters sizes incorrectly - on linux, the tree columns row height is 4px less, and on win it's 1 px less; this is a bug.

Here on Windows 10 it's always 24px tall. On Linux it's hard to get always the correct height as different Linux themes have different heights. On my Ubuntu the height is correct.

(In reply to alta88 from comment #22)

Notes: there's a typo for linux column padding, so that's fixed already.
A padding-left/right: 3px; would work too to centre the icon.
Please check osx - I used dot.svg should it be dot.png or perhaps something else? If something else, please provide an image that's suitable if it doesn't exist already.
I propose to move the check.gif and notchecked.gif to the shared folder and
let it load on all platforms. Then you can use them on Mac too.

Great. Can we (you are probably better) make a checkbox.svg?? This will make everything so much easier

Perhaps it can be 24x for numerous places it could be used, then it can be set to the appropriate width/height.

Attached patch selectColTests.patch (obsolete) — Splinter Review
Attachment #9085217 - Flags: review?(acelists)
Attached image checkbox.svg

Something like this?

Example code on the header:
.selectColumnHeader {
min-width: 28px;
cursor: pointer;
list-style-image: url("chrome://messenger/skin/icons/checkbox.svg");
}

.selectColumnHeader > .treecol-icon {
width: 16px;
height: 16px;
-moz-context-properties: fill, stroke, stroke-opacity;
fill: transparent;
stroke: currentColor;
stroke-opacity: 0;
}

.selectColumnHeader.someselected > .treecol-icon {
stroke-opacity: 0.3;
fill: currentColor;
}

.selectColumnHeader.allselected > .treecol-icon {
fill: currentColor;
}

Attached patch selectCol.patchSplinter Review

Incorporated the icon and rules. Any further css tweaks should go into a followup patch please.

Attachment #9084964 - Attachment is obsolete: true
Attachment #9084964 - Flags: ui-review?(alessandro)
Attachment #9084964 - Flags: review?(acelists)
Attachment #9085329 - Flags: ui-review?(richard.marti)
Attachment #9085329 - Flags: ui-review?(alessandro)
Attachment #9085329 - Flags: review?(acelists)

paenglab, check.svg is in shared but not registered in jar, should it be? it duplicates one in toolkit (global).

Comment on attachment 9085329 [details] [diff] [review]
selectCol.patch

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

::: mail/themes/windows/mail/messageIcons.css
@@ +33,5 @@
> +  fill: currentColor;
> +}
> +
> +treechildren::-moz-tree-image(selectCol) {
> +  margin-inline-start: 6px;

Please remove this line. Then it looks the same as on Linux and Mac.
Attachment #9085329 - Flags: ui-review?(richard.marti) → ui-review+

(In reply to alta88 from comment #29)

paenglab, check.svg is in shared but not registered in jar, should it be? it duplicates one in toolkit (global).

check.svg is not used and could be removed in our tree.

(In reply to Richard Marti (:Paenglab) from comment #30)

Comment on attachment 9085329 [details] [diff] [review]
selectCol.patch

Review of attachment 9085329 [details] [diff] [review]:

::: mail/themes/windows/mail/messageIcons.css
@@ +33,5 @@

  • fill: currentColor;
    +}

+treechildren::-moz-tree-image(selectCol) {

  • margin-inline-start: 6px;

Please remove this line. Then it looks the same as on Linux and Mac.

The checkbox in the rows is centered nicely on linux, but not on win without this. Maybe because I'm just editing the style file in devtools to simulate win. So ok.

(In reply to Richard Marti (:Paenglab) from comment #31)

(In reply to alta88 from comment #29)

paenglab, check.svg is in shared but not registered in jar, should it be? it duplicates one in toolkit (global).

check.svg is not used and could be removed in our tree.

I'll remove it in a final patch for checkin here.

Attached image checkbox-margin.png

This is how it looks on my Win 10 with the margin.

(In reply to Richard Marti (:Paenglab) from comment #33)

Created attachment 9085496 [details]
checkbox-margin.png

This is how it looks on my Win 10 with the margin.

Hmm, I'm testing with win7. What to do?

Edit: Yes, it could be an @media rule in mailWindow1.css, but that isn't part of the SearchDialog.xul. I had a version of the patch which added it there and fixed up the css in .xul, but then all the <menu> overrides broke the look of the search <menu>s. So all these rules are duplicated in messageIcons.css.

I guess it'll be an @media rule in win messageIcons.css for win7 and win8, right?

Checked on my Win7 VM and it looks the same with Basic and Classic. With removed margin it's looking correct.

Ok, I'll just remove it from win.

Comment on attachment 9085329 [details] [diff] [review]
selectCol.patch

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

Looks almost good, with some minor adjustments.
Feel free to assign ui-r+ after these changes.

For a follow up bug, we should consider moving the repeated CSS intro a shared file, just to keep it in mind.
Also, I still believe the usage of the checkboxes introduces a new paradigm by itself, that's why I think it may be necessary to maintain the messages selected via checkobox selected if the user clicks on another message row.
We can explore this UX on a follow up bug.

As usual, great work!

::: mail/themes/windows/mail/messageIcons.css
@@ +16,5 @@
> +
> +.selectColumnHeader > .treecol-icon,
> +treechildren::-moz-tree-image(selectCol) {
> +  width: 16px;
> +  height: 16px;

Make this 12px in size. 16px is too big and looks unbalanced.

@@ +25,5 @@
> +}
> +
> +.selectColumnHeader.someselected > .treecol-icon {
> +  stroke-opacity: 0.3;
> +  fill: currentColor;

To create a better visual separation between the 3 states, we should use a horizontal stroke when some are selected.
I'll attach the SVG in the next message.

@@ +35,5 @@
> +
> +treechildren::-moz-tree-image(selectCol) {
> +  margin-inline-start: 6px;
> +  width: 12px;
> +  height: 12px;

Make this a 10px square to better fit with the text size of message row.
Attachment #9085329 - Flags: ui-review?(alessandro) → ui-review-

We already have an horizontal SVG we can use for the some selected state:
url("chrome://messenger/skin/icons/hline.svg");

Attached patch selectColCSSTweaks.patch (obsolete) — Splinter Review
Attachment #9085530 - Flags: review?(richard.marti)
Attachment #9085530 - Flags: review?(alessandro)
Attachment #9085329 - Flags: ui-review- → ui-review+

For the record, a horizontal line does not impart a meaning anywhere close to 'some selected', the way the original patch and paenglab's tweaks did. That, to me, means negation or undefined or initially unselected. And in the original extension and even here, the idea was for touchscreens to be supported, and for accessibility (dependent bug). Tiny images defeat both, unless Tb intends to support Touch Density the way Fx does. But time to move on.

Attached patch selectColCSSTweaks.patch (obsolete) — Splinter Review
Attachment #9085530 - Attachment is obsolete: true
Attachment #9085530 - Flags: review?(richard.marti)
Attachment #9085530 - Flags: review?(alessandro)
Attachment #9085532 - Flags: review?(richard.marti)
Attachment #9085532 - Flags: review?(alessandro)
Comment on attachment 9085532 [details] [diff] [review]
selectColCSSTweaks.patch

r+ for the code. Thanks to remove check.svg
I'm not sure we should use the hline as a partly selected symbol. I think the checkbox with the background set is better. Also the checkbox size of 10px on the treechildren looks to me too small. And on the treeheader 14px would for me a good compromise.

But let's decide our UX pro.
Attachment #9085532 - Flags: review?(richard.marti) → review+

To add: the vastly normal state is 'someselected' with 1 being the vast majority of that, so it makes most sense to use the row indicator (normal checked box or check). So none and all are far edge cases; indicator for none is easy, empty box; indicator for all should include an aspect of 'some', or be a clear indicator of all, like maybe the select all icon.

Attached image some-selected.png

Sorry, I should have specified my recommendation better.
The hline should be used inside the checkbox border, not replacing it entirely.
As you can see from the screenshot attached, various applications handle the "some selected" in a couple of ways, like the horizontal line, or a filled inset background.
The hline indicates also the ability to unselect all, which it should be the default behaviour of the header checkbox if some are selected.

Attached image checkbox.svg

Okay, here's the checkbox with a horizontal stroke in it.

The CSS looks like this:

.selectColumnHeader > .treecol-icon,
treechildren::-moz-tree-image(selectCol) {
width: 14px;
height: 14px;
-moz-context-properties: fill, fill-opacity, stroke, stroke-opacity;
fill: currentColor;
stroke: currentColor;
fill-opacity: 0;
stroke-opacity: 0;
}

.selectColumnHeader.someselected > .treecol-icon {
stroke-opacity: 1;
}

.selectColumnHeader.allselected > .treecol-icon,
treechildren::-moz-tree-image(selectCol, selected) {
fill-opacity: 1;
}

treechildren::-moz-tree-image(selectCol) {
width: 12px;
height: 12px;
}

Note: I increased the checkbox size a bit like I think it could work.

Attached patch selectColCSSTweaks.patch (obsolete) — Splinter Review

updated tweaks.

Attachment #9085532 - Attachment is obsolete: true
Attachment #9085532 - Flags: review?(alessandro)
Attachment #9085756 - Flags: review?(richard.marti)
Attachment #9085756 - Flags: review?(alessandro)
Comment on attachment 9085756 [details] [diff] [review]
selectColCSSTweaks.patch

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

Thanks.

::: mail/themes/linux/mail/messageIcons.css
@@ +33,5 @@
> +treechildren::-moz-tree-image(selectCol, selected) {
> + fill-opacity: 1;
> +}
> +
> +  treechildren::-moz-tree-image(selectCol) {

No indentation.
Attachment #9085756 - Flags: review?(richard.marti) → review+
Comment on attachment 9085756 [details] [diff] [review]
selectColCSSTweaks.patch

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

Great, I like it, and I'm fine with bumping the size like suggested by Richard.
r+
Attachment #9085756 - Flags: review?(alessandro) → review+
Comment on attachment 9085217 [details] [diff] [review]
selectColTests.patch

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

::: mail/base/content/threadPane.js
@@ +150,5 @@
>    }
>  }
>  
>  function HandleSelectColClick(event, row) {
> +console.log("HandleSelectColClick: row - "+row);

To be removed.

::: mail/test/mozmill/folder-display/test-columns.js
@@ +47,5 @@
>  
>    useCorrespondent =
>      Services.prefs.getBoolPref("mail.threadpane.use_correspondents");
>    INBOX_DEFAULTS = [
> +    "selectCol",

Wait, is the new column now visible for some folders by default? It does not seem the test sets them up using this array, just checks if they are already shown per this list.
Was it decided that this column is shown in all new folders?

::: mail/test/mozmill/shared-modules/test-folder-display-helpers.js
@@ +996,5 @@
>  
>  /**
> + * Pretend we are clicking on a row in the select column with our mouse.
> + *
> + * @param aViewIndex If >= 0, the view index provided, if < 0, a reference to

2 spaces after argument name please.

@@ +1007,5 @@
> + */
> +function select_column_click_row(aViewIndex, aController) {
> +  if (aController == null)
> +    aController = mc;
> +  let hasMessageDisplay = "messageDisplay" in aController;

I'd prefer bracket around the 'in' operator expression.
Attachment #9085217 - Flags: review?(acelists) → review+
Comment on attachment 9085329 [details] [diff] [review]
selectCol.patch

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

Yes the square checkboxes look much better now, thanks.

::: mail/base/content/folderDisplay.js
@@ +403,5 @@
>     *  column in this list is checked against |COLUMN_DEFAULT_TESTERS| to see if
>     *  it is actually an appropriate default for the folder type.
>     */
>    DEFAULT_COLUMNS: [
> +    "selectCol",

Here it is. Was this explicitly decided by UX to be shown by default?
Attachment #9085329 - Flags: review?(acelists) → review+

(In reply to :aceman from comment #50)

Here it is. Was this explicitly decided by UX to be shown by default?

I'm against to show it by default.

Indeed, it shouldn't be visible by default.
It wasn't when I tested it and I had to enable that extra column.

Was it decided that this column is shown in all new folders?

It is certainly not added to any existing configuration. It must be explicitly user enabled.

But it has been added to the default set for new subfolders and new profiles. Remove? Why isn't this a general feature that many will appreciate?

Flags: needinfo?(alessandro)

I personally prefer to have it as an opt-in option for the following reasons:

  • The checkbox takes up a bit of space on an already tight message row, better not showing it by default.
  • Our user base is already acquainted with the CTRL+click paradigm for multiple selections.
  • It's always better to slowly ease in UX changes.

This is a great opportunity to do a quick survey after this feature has been out for a while, and consider hanging it from opt-in to opt-out.

Flags: needinfo?(alessandro)

Regarding opt-in vs opt-out, one thing we did for TB3 (or maybe 3.1? It's been a while...) was to have a little walkthrough of new features and let users pick whether they want the new behavior or to stick with the old. Since we're only updating with new features about once a year, we could probably provide a walkthrough like this without being too annoying. It'd be extra work to design the walkthrough, but it could also strike a nice balance between ensuring people are aware of the new feature and not being disruptive to users who are happy with how things are already.

updated for no default.

Attachment #9085756 - Attachment is obsolete: true
Attachment #9086153 - Flags: review+

updated.

Attachment #9085217 - Attachment is obsolete: true
Attachment #9086157 - Flags: review+

kind sheriff, perhaps a try run?

Keywords: checkin-needed

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/51edb6fe5567
Allow selecting messages via select column instead of classic selection. r=aceman, ui-r=Paenglab
https://hg.mozilla.org/comm-central/rev/ac38dd59cf09
additional selectCol CSS tweaks. r=Paenglab,aleca
https://hg.mozilla.org/comm-central/rev/8bcc3b8b7c6f
Tests for addition of selectCol. r=aceman

Status: NEW → RESOLVED
Closed: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0
Regressions: 1574673
No longer regressions: 1574673
Duplicate of this bug: 1574673
Attached patch fix-mochi.patchSplinter Review

Fix the broken Mochitest, see bug 1574673.

Attachment #9086208 - Flags: review?(geoff)

https://hg.mozilla.org/comm-central/rev/a67968a219c595706b9f01ccb71ce601c5001615
Follow-up: Adjust Mochitest browser_ext_menus.js::test_thread_pane. rs=bustage-fix CLOSED TREE DONTBUILD

If landed it to fix the test. I don't know why it was ever a good idea to click right in the corner at (0, 0).

Comment on attachment 9086208 [details] [diff] [review]
fix-mochi.patch

A closer look at the Try run next time, perhaps?
Attachment #9086208 - Flags: review?(geoff) → review+

Yes, but please stop all those Mochitests from failing most of the time ;-) - There are so many and inconsistent failures.

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