Closed Bug 527595 Opened 12 years ago Closed 3 years ago

No border around focused subject (since new wrapping subject)

Categories

(Thunderbird :: Message Reader UI, defect)

defect
Not set
minor

Tracking

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

RESOLVED FIXED
Thunderbird 63.0
Tracking Status
thunderbird_esr60 --- fixed
thunderbird61 --- wontfix
thunderbird62 --- fixed
thunderbird63 --- fixed

People

(Reporter: BenB, Assigned: Paenglab)

Details

(Keywords: access, regression)

Attachments

(3 files, 7 obsolete files)

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>
Attached patch Fix, v1 (obsolete) — Splinter Review
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?
Attached image Screenshot with Fix v1 (obsolete) —
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).
Attachment #411328 - Flags: ui-review?
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
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
Attachment #411332 - Attachment is obsolete: false
Attachment #411328 - Attachment is obsolete: false
Attachment #411328 - Flags: ui-review?(clarkbw)
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)
Attached image 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.
Attached patch Fix, v4 (obsolete) — Splinter Review
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)
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+
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.
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?
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.
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)
Attached patch Bug527595.patch (obsolete) — Splinter Review
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
Attached patch Bug527595.patchSplinter Review
(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 on attachment 8991700 [details] [diff] [review]
Bug527595.patch

Works for me, thanks. Uplift?
Attachment #8991700 - Flags: review?(jorgk) → review+
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?
Keywords: checkin-needed
Attachment #8991700 - Flags: approval-comm-esr60?
Attachment #8991700 - Flags: approval-comm-esr60+
Attachment #8991700 - Flags: approval-comm-beta?
Attachment #8991700 - Flags: approval-comm-beta+
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
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
You need to log in before you can comment on or make changes to this bug.