No border around focused subject (since new wrapping subject)

RESOLVED FIXED in Thunderbird 63.0

Status

--
minor
RESOLVED FIXED
9 years ago
5 months ago

People

(Reporter: BenB, Assigned: Paenglab)

Tracking

({access, regression})

Thunderbird 63.0
access, regression

Thunderbird Tracking Flags

(thunderbird_esr60 fixed, thunderbird61 wontfix, thunderbird62 fixed, thunderbird63 fixed)

Details

Attachments

(3 attachments, 7 obsolete attachments)

(Reporter)

Description

9 years ago
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>
(Reporter)

Comment 1

9 years ago
Created attachment 411328 [details] [diff] [review]
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.
Attachment #411328 - Flags: ui-review?
(Reporter)

Comment 2

9 years ago
Created attachment 411332 [details]
Screenshot with Fix v1
(Reporter)

Comment 3

9 years ago
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).
(Reporter)

Updated

9 years ago
Attachment #411328 - Flags: ui-review?
Created attachment 411489 [details]
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!
Attachment #411328 - Attachment is obsolete: true
Attachment #411332 - Attachment is obsolete: true
Attachment #411489 - Flags: ui-review?(clarkbw)
Attachment #411489 - Flags: ui-review?(clarkbw) → ui-review-
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
(Reporter)

Comment 6

9 years ago
Created attachment 411493 [details] [diff] [review]
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).
Assignee: nobody → ben.bucksch
(Reporter)

Updated

9 years ago
Attachment #411332 - Attachment is obsolete: false
(Reporter)

Updated

9 years ago
Attachment #411328 - Attachment is obsolete: false
Attachment #411328 - Flags: ui-review?(clarkbw)
(Reporter)

Comment 7

9 years ago
Created attachment 411494 [details] [diff] [review]
Fix, v3 - dotted border using outline

Bryan, outline works beautifully here, thanks!
This patch uses that.
Attachment #411328 - Attachment is obsolete: true
Attachment #411332 - Attachment is obsolete: true
Attachment #411489 - Attachment is obsolete: true
Attachment #411493 - Attachment is obsolete: true
Attachment #411494 - Flags: ui-review?(clarkbw)
Attachment #411494 - Flags: review?(bwinton)
Attachment #411328 - Flags: ui-review?(clarkbw)
(Reporter)

Comment 8

9 years ago
Created attachment 411495 [details]
Screenshot with Fix v3
What about adding a "-moz-outline-radius", to make it nice and round, like the border?
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.
(Reporter)

Comment 11

9 years ago
Created attachment 411503 [details] [diff] [review]
Fix, v4

With -moz-outline-radius: 2px; as requested.
Attachment #411494 - Attachment is obsolete: true
Attachment #411503 - Flags: ui-review?(clarkbw)
Attachment #411503 - Flags: review?(bwinton)
Attachment #411494 - Flags: ui-review?(clarkbw)
Attachment #411494 - Flags: review?(bwinton)
(Reporter)

Comment 12

9 years ago
Blake, yes, will do in next patch. With that, is that r=bwinton?
Bryan, fine with that?
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?
(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 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.
Attachment #411503 - Flags: review?(bwinton) → review+
Created attachment 411511 [details]
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
- 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.
(Reporter)

Comment 18

9 years ago
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.
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.
(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)...
Keywords: access
Summary: [Accessibility] No border around focused subject (since new wrapping subject) → No border around focused subject (since new wrapping subject)
Comment on attachment 411503 [details] [diff] [review]
Fix, v4

sorry, didn't see ben's comment 18 there
Attachment #411503 - Flags: ui-review?(clarkbw) → ui-review+
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?
(Reporter)

Comment 23

9 years ago
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.
(Reporter)

Updated

9 years ago
Assignee: ben.bucksch → nobody
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?
perhaps richard will know
Flags: needinfo?(richard.marti)
(Assignee)

Comment 26

5 months ago
Created attachment 8991429 [details] [diff] [review]
Bug527595.patch

Updated patch.

Thomas, can you try this?
Attachment #411503 - Attachment is obsolete: true
Flags: needinfo?(richard.marti)
Attachment #8991429 - Flags: feedback?(bugzilla2007)
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.
Attachment #8991429 - Flags: ui-review+
Attachment #8991429 - Flags: feedback?(bugzilla2007)
Attachment #8991429 - Flags: feedback+
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
(Assignee)

Comment 28

5 months ago
Created attachment 8991700 [details] [diff] [review]
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.
Attachment #8991429 - Attachment is obsolete: true
Attachment #8991700 - Flags: ui-review+
Attachment #8991700 - Flags: review?(jorgk)
(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.
(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

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

Works for me, thanks. Uplift?
Attachment #8991700 - Flags: review?(jorgk) → review+
(Assignee)

Comment 32

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

It should apply on ESR60 without change.
Attachment #8991700 - Flags: approval-comm-esr60?
Attachment #8991700 - Flags: approval-comm-beta?
(Assignee)

Updated

5 months ago
Keywords: checkin-needed

Updated

5 months ago
Attachment #8991700 - Flags: approval-comm-esr60?
Attachment #8991700 - Flags: approval-comm-esr60+
Attachment #8991700 - Flags: approval-comm-beta?
Attachment #8991700 - Flags: approval-comm-beta+

Comment 33

5 months ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2c6eadfca726
Set a outline for focused header fields like for the .emailDisplayButton. r=jorgk, ui-r=ThomasD
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

5 months ago
Target Milestone: --- → Thunderbird 63.0

Comment 34

5 months ago
Beta (TB 62):
https://hg.mozilla.org/releases/comm-beta/rev/aaaedc391ec685b8f85e282d45adfc6bd77eb507
status-thunderbird61: --- → wontfix
status-thunderbird62: --- → fixed
status-thunderbird63: --- → fixed
status-thunderbird_esr60: --- → affected
You need to log in before you can comment on or make changes to this bug.