Closed Bug 955083 Opened 10 years ago Closed 10 years ago

accessibility: Make it clear that topic control in conversation info toolbar is actionable

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: aleth)

Details

Attachments

(1 file, 3 obsolete files)

*** Original post on bio 1654 by James Teh <jamie AT nvaccess.org> at 2012-08-19 23:41:00 UTC ***

Currently, the topic control (@class="statusMessage") in the conversation info toolbar (@class="conv-top-info") has an accessible role of label. From this, it isn't clear to an assistive technology user that this can be clicked to edit the topic. An accessible role of button would be more appropriate. This can be achieved (without affecting anything else) using @role="button".

Ideally, this role wouldn't be set unless the topic really is editable. However, I'm not sure if there's a way to tell whether it's editable for sighted users either.
*** Original post on bio 1654 at 2012-08-20 00:13:50 UTC ***

(In reply to comment #0)
> However, I'm not sure if there's a way to tell whether it's editable for
> sighted users either.

For sighted users, the mouse cursor changes when over the topic if it is editable (to indicate that you can click it).
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1654 as attmnt 1820 at 2012-08-21 21:06:00 UTC ***

I can't test this, but it should work I think.
Attachment #8353579 - Flags: review?(florian)
*** Original post on bio 1654 at 2012-08-21 21:17:42 UTC ***

(In reply to comment #2)
> I can't test this, but it should work I think.

Actually, on second thoughts, I'm not so sure. James, will @role="button" get in the way while editing (i.e. after the "button" is pressed)?
*** Original post on bio 1654 at 2012-08-21 21:26:48 UTC ***

Also, does the same problem arise for the display name and status in the contact list?
*** Original post on bio 1654 at 2012-08-21 21:39:00 UTC ***

(In reply to comment #4)
> Also, does the same problem arise for the display name and status in the
> contact list?

Very likely. Imitating what's been done in https://bugzilla.mozilla.org/show_bug.cgi?id=778709 may be a good idea for the status selector.
*** Original post on bio 1654 at 2012-08-21 21:44:39 UTC ***

(In reply to comment #5)
> Very likely. Imitating what's been done in
> https://bugzilla.mozilla.org/show_bug.cgi?id=778709 may be a good idea for the
> status selector.

I don't understand how that would help for this bug? That patch seems much closer to what I did in bug 955071 (bio 1642) (awaiting review).
*** Original post on bio 1654 by James Teh <jamie AT nvaccess.org> at 2012-08-21 21:45:33 UTC ***

(In reply to comment #3)
> James, will @role="button" get
> in the way while editing (i.e. after the "button" is pressed)?
Ah, I didn't realise it was the same control for both. Yes, the role will interfere while editing. It should be removed just before the editable text field becomes active.
Comment on attachment 8353579 [details] [diff] [review]
Patch

*** Original change on bio 1654 attmnt 1820 at 2012-08-21 21:57:17 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353579 - Flags: review?(florian) → review-
*** Original post on bio 1654 at 2012-08-21 21:57:43 UTC ***

(In reply to comment #6)
> (In reply to comment #5)
> > Very likely. Imitating what's been done in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=778709 may be a good idea for the
> > status selector.
> 
> I don't understand how that would help for this bug?

It contains the changes required to make the status selector usable with only the keyboard.

More specifically, it makes the focus on the status selector visible (important when tabbing through the controls of the window) and it lets the user open the status type popup with the down arrow while the status selector is focused.

> That patch seems much
> closer to what I did in bug 955071 (bio 1642) (awaiting review).

It also contains a part similar to that, yes.

That bmo bug was more or less a catch all for issues contributing to the "the status selector of the chat toolbar sucks" impression ;).
*** Original post on bio 1654 at 2012-08-21 22:03:48 UTC ***

(In reply to comment #8)
> It contains the changes required to make the status selector usable with only
> the keyboard.

Ah OK, I meant the status message, not the status selector. The status selector should probably have these improvements, and making the focus visible will be wanted for all of the contact list I think, but that's a separate bug.
*** Original post on bio 1654 at 2012-08-21 22:19:49 UTC ***

(In reply to comment #9)
> (In reply to comment #8)
> > It contains the changes required to make the status selector usable with only
> > the keyboard.
> 
> Ah OK, I meant the status message, not the status selector.

What I'm calling "status selector" is the set of controls that lets the user change the status, ie the status type button and popup + the status message editor.
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1654 as attmnt 1826 at 2012-08-22 10:02:00 UTC ***

Leaving the potential missing roles in the contacts window for another bug - maybe James could confirm if/that they are also required there?
Attachment #8353585 - Flags: review?(florian)
Comment on attachment 8353579 [details] [diff] [review]
Patch

*** Original change on bio 1654 attmnt 1820 at 2012-08-22 10:02:31 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353579 - Attachment is obsolete: true
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8353585 [details] [diff] [review]
Patch

*** Original change on bio 1654 attmnt 1826 at 2012-08-22 10:12:00 UTC ***

Ok with me.

Nit: you indented the content of the CDATA sections of the new editing property with one more space than the other existing CDATA sections around.
Attachment #8353585 - Flags: review?(florian) → review+
Attached patch Patch with fixed indent (obsolete) — Splinter Review
*** Original post on bio 1654 as attmnt 1827 at 2012-08-22 10:32:00 UTC ***

One day I should file a patch that fixes the odd-#-of-spaces indentation of certain xml files...
Attachment #8353586 - Flags: review+
Comment on attachment 8353585 [details] [diff] [review]
Patch

*** Original change on bio 1654 attmnt 1826 at 2012-08-22 10:32:14 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353585 - Attachment is obsolete: true
Whiteboard: [checkin-needed]
*** Original post on bio 1654 at 2012-08-23 01:29:01 UTC ***

$ patch -p5 < ../../attachment.cgi\?id\=1827
patching file conversation.xml
Hunk #1 FAILED at 1623.
Hunk #2 succeeded at 1751 (offset 16 lines).
Hunk #3 succeeded at 1854 (offset 16 lines).
1 out of 3 hunks FAILED -- saving rejects to file conversation.xml.rej
Attached patch PatchSplinter Review
*** Original post on bio 1654 as attmnt 1830 at 2012-08-23 09:01:00 UTC ***

That's what bug 955089 (bio 1660) was for ;)
Attachment #8353589 - Flags: review+
Comment on attachment 8353586 [details] [diff] [review]
Patch with fixed indent

*** Original change on bio 1654 attmnt 1827 at 2012-08-23 09:01:48 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353586 - Attachment is obsolete: true
*** Original post on bio 1654 at 2012-08-28 00:56:21 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/0ed1a0d680a4 Thanks for fixing this!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
You need to log in before you can comment on or make changes to this bug.