527595 - No border around focused subject (since new wrapping subject)

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Ben Bucksch (:BenB)]

Minor regression caused by bug 489609.

Reproduction:
1. Click on the message header pane, in an empty area
2. Click Tab (on your keyboard) once.
   The From address should have a dotted border.
   (If you'd press the Context-Menu keyboard key (left to the right Ctrl key),
   you should get the context menu of the email address. But don't do that now.)
3. Click Tab again. Now the Star next to the From address should have a border.
4. Click Tab again. Now the subject should be focused and have a
   dotted border.
5. Click the context menu key, select Copy, paste in some other app.

Actual result:
Step 4: No dotted border around subject, no other visual indication for focus either.
Step 5: The subject is copied.
I.e. The tab and focus works, but there is no visual indication.

Expected result:
Step 4: dotted border around subject.

Same for Organization: header and similar, BTW.

Bug reported by Thomas D. <...duellmann24.net>

Comment 1

[Ben Bucksch (:BenB)]

Attachment: Fix, v1

Fix is straight-forward: Just make a CSS rule for :focus.

Gotcha: border doesn't work! (No idea why.) Using light gray background instead.

Comment 2

[Ben Bucksch (:BenB)]

Attachment: Screenshot with Fix v1

Comment 3

[Ben Bucksch (:BenB)]

I tried using CSS system color Highlight[Text], like text selection, but that doesn't work well when the user really want to select a part of the text (even when just with the mouse).

Comment 4

[Thomas D. (:thomas8)]

Attachment: Screenshot 2: dotted, rounded blue border for subject on focus (same as contacts-focus)

I think just having blue, rounded, dotted border as we now have when focussing contacts via tab would be nice and consistent as an interim solution (until we're able to restore all the details of keyboard selection options that TB2 inherited from the former xul:textbox, but we now have xul:description).

Ben, could you try this (tested working on Win XP):

http://mxr.mozilla.org/comm-1.9.1/source/mail/themes/qute/mail/messageHeader.css#594

mail-headerfield > .headerValue {
  -moz-user-focus: normal;
  -moz-user-select: text;
+ border: 1px dotted transparent; /* needed to avoid moving headers on focus */
+ -moz-border-radius: 2px; /* nice round corners */
}

+ mail-headerfield > .headerValue:focus {
+   border-color: Highlight; /* on focus, we only change color of existing transparent border */
}

Thanks for looking into this!

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 411489 [details]
Screenshot 2: dotted, rounded blue border for subject on focus (same as contacts-focus)

you should try outline [1] instead of border so you don't need to do the transparent 1px hack.  And the Highlight color should work as posted.

[1] https://developer.mozilla.org/en/CSS/outline

Comment 6

[Ben Bucksch (:BenB)]

Attachment: Fix, v2, from Thomas, with dotted border.

This is Thomas' patch above, as proper patch.

Unfortunately, it does not work for me. I see no border. Not when I change color to "black" either. I tried something similar yesterday myself and it didn't work then either. I have no idea why. I'm on Linux, SuSE 10.3, KDE with GTK-Qt theme (maps Qt theme to GTK theme).

Comment 7

[Ben Bucksch (:BenB)]

Attachment: Fix, v3 - dotted border using outline

Bryan, outline works beautifully here, thanks!
This patch uses that.

Comment 8

[Ben Bucksch (:BenB)]

Attachment: Screenshot with Fix v3

Comment 9

[Blake Winton (:bwinton) (:☕️)]

What about adding a "-moz-outline-radius", to make it nice and round, like the border?

Comment 10

[Blake Winton (:bwinton) (:☕️)]

Oh, and I previously suggested to Andreas that when he used a :hover, he should add in a similar rule with a :focus, so I'll suggest the reverse to you, giving us:

mail-headerfield > .headerValue:hover,
mail-headerfield > .headerValue:focus {
  outline: 1px dotted Highlight;
  -moz-outline-radius: 2px;
}

Thoughts?

Later,
Blake.

Comment 11

[Ben Bucksch (:BenB)]

Attachment: Fix, v4

With -moz-outline-radius: 2px; as requested.

Comment 12

[Ben Bucksch (:BenB)]

Blake, yes, will do in next patch. With that, is that r=bwinton?
Bryan, fine with that?

Comment 13

[Thomas D. (:thomas8)]

Sorry, but outline pushes the date and the buttons out of view (about 5mm) on the right hand side, for long subjects (> 1 line). I tried that before filing my proposal.

Bryan, could you clarify what you mean by "The Highlight color should work as posted"? Exactly which Highlight color, and posted where?

Comment 14

[Blake Winton (:bwinton) (:☕️)]

(In reply to comment #13)
> Sorry, but outline pushes the date and the buttons out of view (about 5mm) on
> the right hand side, for long subjects (> 1 line). I tried that before filing
> my proposal.

Thomas, I don't see that in the screenshot Ben posted…  Could you post a new screenshot that shows that problem?

Thanks,
Blake.

Comment 15

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 411503 [details] [diff] [review]
Fix, v4

>+mail-headerfield > .headerValue:focus {
>+   outline: 1px dotted Highlight;
>+   -moz-outline-radius: 2px;
>+}

Add the :hover (if it looks good), and change the indentation from 3 spaces to 2 spaces, and you've got an r=me.  ;)

(I'm guessing Bryan will want to address the problem Tom mentioned was before giving it the ui-r+, but the code looks good.)

Later,
Blake.

Comment 16

[Thomas D. (:thomas8)]

Attachment: Screenshot with outline as in fix v4, pushes things off the screen

Differences to screenshot of fix v3:
- Ben's screenshot is from another OS, I have XP
- no rounded borders yet in Ben's screenshot

Comment 17

[Thomas D. (:thomas8)]

- another obvious difference is that ben's screenshot has "View all headers" and a scrollbar, whereas I'm running "View normal headers" and no scrollbar on this one.

Comment 18

[Ben Bucksch (:BenB)]

Thomas, just tested Normal Headers, all is fine, no pushing, moving or jiggling whatsoever when I tab into the subject and the outline border appears.

Blake, re hover: wouldn't it be odd if 2 fields had the dotted border at the same time? Click in subject, and move mouse cursor over Organization. I think we should leave it to :focus, that's the common use.

Comment 19

[Blake Winton (:bwinton) (:☕️)]

So the focus wouldn't follow the mouse?  If not, then yeah, your way makes more sense.  If the focus follows the mouse, then, uh, I guess having :focus would catch it.  Okay, works for me.

Comment 20

[Thomas D. (:thomas8)]

(In reply to comment #18)
> Thomas, just tested Normal Headers, all is fine, no pushing, moving or jiggling
> whatsoever when I tab into the subject and the outline border appears.

On which OS did you test? I'm still seeing this problem on Win XP.

(In reply to comment #19)
> So the focus wouldn't follow the mouse?
No, it doesn't. Sometimes it doesn't even follow mouse /clicks/, and sometimes when it does and there's a clear focus, Bryan thinks people don't understand focus and so we just ignore it and delete whole messages instead of focussed attachments (Bug 315144)...

Comment 21

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 411503 [details] [diff] [review]
Fix, v4

sorry, didn't see ben's comment 18 there

Comment 22

[Thomas D. (:thomas8)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091121 Thunderbird/3.0 (tb3rc1build3)

Bryan, the current patch will break the layout on Windows XP and push things to the right, as in screenshot attachment 411511 [details] (as mentioned in comment 13, and verified today on a clean install without addons and the like).
Using css-"border" instead would not have this problem on Windows XP. Apart from two extra lines of css-code, I don't see anything wrong with the transparent 1px border "hack". As mentioned in bug 460647 comment 11, it will not even add to the total height of the header pane.

Is it OK then if we swap "outline" back to "border" in the patch?

Comment 23

[Ben Bucksch (:BenB)]

Thomas D, please see comment 1:
> border doesn't work! (No idea why.)
and comment 6:
> Thomas' patch above ... Unfortunately, it does not work for me.
> I see no border.

Comment 24

[Thomas D. (:thomas8)]

What's the way forward here?
Can we not use the same method of focus indication that works for surrounding fields like to-recipient where we correctly show the dotted border on focus?

Comment 25

[Wayne Mery (:wsmwk)]

perhaps richard will know

Comment 26

[Richard Marti (:Paenglab)]

Attachment: Bug527595.patch

Updated patch.

Thomas, can you try this?

Comment 27

[Thomas D. (:thomas8)]

Comment on attachment 8991429 [details] [diff] [review]
Bug527595.patch

(In reply to Richard Marti (:Paenglab) from comment #26)
> Created attachment 8991429 [details] [diff] [review]
> Thomas, can you try this?

Yes, this works for me on Windows 10. I'm no longer using Windows XP where the old patches were failing.
It's a bit odd that the focus border spans a lot of whitespace at the right of the actual subject text, but that's probably harder to fix as it involves the XUL elements used. For comparison, recipients' focus border is not including any whitespace.

OT: There is an invisible tab stop after the last header stop (blue star of recipient), which does not do anything afasics and should probably be removed.

Comment 28

[Richard Marti (:Paenglab)]

Attachment: Bug527595.patch

(In reply to Thomas D. (currently busy elsewhere) from comment #27)
> Comment on attachment 8991429 [details] [diff] [review]
> Bug527595.patch
> 
> (In reply to Richard Marti (:Paenglab) from comment #26)
> > Created attachment 8991429 [details] [diff] [review]
> > Thomas, can you try this?
> 
> Yes, this works for me on Windows 10. I'm no longer using Windows XP where
> the old patches were failing.
> It's a bit odd that the focus border spans a lot of whitespace at the right
> of the actual subject text, but that's probably harder to fix as it involves
> the XUL elements used. For comparison, recipients' focus border is not
> including any whitespace.

There is no child below headerValue which could be used for the focus ring, so it uses always the whole width. I'm using now outline-offset: -1px; to make it 2px smaller and that the focus rings don't touch each other when tabbing through the full header list.

> OT: There is an invisible tab stop after the last header stop (blue star of
> recipient), which does not do anything afasics and should probably be
> removed.

I don't know where it goes in this tab step.

Comment 29

[Thomas D. (:thomas8)]

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

> > OT: There is an invisible tab stop after the last header stop (blue star of
> > recipient), which does not do anything afasics and should probably be
> > removed.
> 
> I don't know where it goes in this tab step.

It's the date header, and it should get a focus ring at least. Unfortunately, it's a dead end, nothing which you can do here with keyboard except "Customize" (which is kinda useful because it's not otherwise keyboard accessible afasics).
Yeah, message header XUL is a can of worms and it truly sucks. Even in subject, you can't even keyboard-cursor because it's not a textbox, but a description (not sure why descriptions are mouse-selectable, but not keyboard-selectable, that looks like another XUL bug). Also, Ctrl+C on focused subject does nothing, which is inconsistent as we have a context menu which allows copying. There must be bugs on record somewhere.

Comment 30

[Thomas D. (:thomas8)]

(In reply to Thomas D. (currently busy elsewhere) from comment #29)
> It's the date header, and it should get a focus ring at least.
> Unfortunately, it's a dead end, nothing which you can do here with keyboard
> except "Customize" (which is kinda useful because it's not otherwise
> keyboard accessible afasics).

To be clear, it's a bug that date header is not keyboard accessible, because users may want to copy it.

Comment 31

[Jorg K (CEST = GMT+2)]

Comment on attachment 8991700 [details] [diff] [review]
Bug527595.patch

Works for me, thanks. Uplift?

Comment 32

[Richard Marti (:Paenglab)]

Comment on attachment 8991700 [details] [diff] [review]
Bug527595.patch

It should apply on ESR60 without change.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2c6eadfca726
Set a outline for focused header fields like for the .emailDisplayButton. r=jorgk, ui-r=ThomasD

Comment 34

[Jorg K (CEST = GMT+2)]

Beta (TB 62):
https://hg.mozilla.org/releases/comm-beta/rev/aaaedc391ec685b8f85e282d45adfc6bd77eb507

Comment 35

[Jorg K (CEST = GMT+2)]

TB 60 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/0e8b178e9c916074bf6eab039ac9cd83187f720d