Closed
Bug 900097
Opened 11 years ago
Closed 11 years ago
statusbarpanel shouldn't be a button accessible
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: surkov, Assigned: maxli)
References
Details
(Keywords: access)
Attachments
(2 files, 1 obsolete file)
2.95 KB,
patch
|
enndeakin
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.65 KB,
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
statusbarpanel is exposed as a button accessible (see bug 419867), now it doesn't seem anything in common with buttons (neither visually neither by usage (for example error console uses it as a container http://mxr.mozilla.org/mozilla-central/source/toolkit/components/console/content/console.xul#95). Besides confusing accessibility (like button containing a textfield) it's caused bug 888981. MSD claims that status bar was removed (https://developer.mozilla.org/en-US/docs/XUL/statusbarpanel) so perhaps all status bar related stuff are just a rudiment.
Comment 1•11 years ago
|
||
surkov, is this a blocker for resolution of bug 888981? If so, please set the tracking-firefox24 flag to ?
Flags: needinfo?(surkov.alexander)
Reporter | ||
Updated•11 years ago
|
tracking-firefox24:
--- → ?
Flags: needinfo?(surkov.alexander)
Reporter | ||
Comment 2•11 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #1) > surkov, is this a blocker for resolution of bug 888981? If so, please set > the tracking-firefox24 flag to ? yes, it should be an easy (and less risky) fix for that bug.
Reporter | ||
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Updated•11 years ago
|
status-firefox24:
--- → affected
Reporter | ||
Comment 3•11 years ago
|
||
Enn, can you advise us how to proceed here?
Comment 4•11 years ago
|
||
statusbarpanel was intended to be a type of button, but an xbl issue (bug 119389) means that it only becomes a button when either class="statusbarpanel-iconic" or class="statusbarpanel-iconic-text" is used. For compatibility I'd suggest making it only use a button accessible in those cases.
Flags: needinfo?(enndeakin)
Comment 5•11 years ago
|
||
(In reply to alexander :surkov from comment #3) > Enn, can you advise us how to proceed here? Tracking this back, as I think this was not intentional.
status-firefox24:
--- → affected
Reporter | ||
Comment 6•11 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #5) > (In reply to alexander :surkov from comment #3) > > Enn, can you advise us how to proceed here? > > Tracking this back, as I think this was not intentional. right, thank you, presumably didn't hit f5 before sending the comment, due to some reason bugzilla didn't alert about collision
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → maxli
Assignee | ||
Comment 7•11 years ago
|
||
Attachment #787412 -
Flags: review?(enndeakin)
Comment 8•11 years ago
|
||
Comment on attachment 787412 [details] [diff] [review] Patch >+ let classAttr = this.getAttribute('class'); >+ return classAttr === 'statusbarpanel-iconic' || >+ classAttr === 'statusbarpanel-iconic-text' ? Use this.classList.contains. (Also, use double quotes.)
Reporter | ||
Comment 9•11 years ago
|
||
Wouldn't it be nicer to back out bug 419867 instead? Should statusbarpanel binding have display="xul:button" attribute?
Comment 10•11 years ago
|
||
(In reply to alexander :surkov from comment #9) > Wouldn't it be nicer to back out bug 419867 instead? Should statusbarpanel > binding have display="xul:button" attribute? you'd need to add a nsIAccessibleProvider to the statusbarpanel-iconic-text binding as well, but please do this it'll make bug 846185 easier.
Comment 11•11 years ago
|
||
Comment on attachment 787412 [details] [diff] [review] Patch r- for last comment please have bindings always return the same value.
Attachment #787412 -
Flags: review?(enndeakin) → review+
Comment 12•11 years ago
|
||
(In reply to alexander :surkov from comment #9) > Wouldn't it be nicer to back out bug 419867 instead? Should statusbarpanel > binding have display="xul:button" attribute? Yes, you should do this instead. The two bindings that are button-type should use button accessibleTypes.
Comment 13•11 years ago
|
||
Comment on attachment 787412 [details] [diff] [review] Patch The comment suggests you wanted to mark this -.
Attachment #787412 -
Flags: review+ → review-
Comment 14•11 years ago
|
||
(In reply to Neil Deakin from comment #13) > Comment on attachment 787412 [details] [diff] [review] > Patch > > The comment suggests you wanted to mark this -. hah, yes :/
Assignee | ||
Comment 15•11 years ago
|
||
Attachment #787412 -
Attachment is obsolete: true
Attachment #787806 -
Flags: review?(enndeakin)
Reporter | ||
Comment 16•11 years ago
|
||
Also, it'd be nice to have some a11y tests (role/test_general.xul looks like a good place for this)
Updated•11 years ago
|
Attachment #787806 -
Flags: review?(enndeakin) → review+
Reporter | ||
Comment 17•11 years ago
|
||
Neil, what about display="xul:button"?
Comment 18•11 years ago
|
||
It's ignore on the base statusbarpanel so you could remove it if you wanted, but it is used on the others.
Assignee | ||
Comment 19•11 years ago
|
||
Attachment #787862 -
Flags: review?(surkov.alexander)
Assignee | ||
Comment 20•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce71b5adcb79
Comment 21•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ce71b5adcb79
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Updated•11 years ago
|
Summary: searchbarpanel shouldn't be a button accessible → statusbarpanel shouldn't be a button accessible
Assignee | ||
Comment 22•11 years ago
|
||
Comment on attachment 787806 [details] [diff] [review] Patch v2 [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 722265 User impact if declined: Accessibility users will experience browser hangs when using certain addons like DownThemAll or by opening the error console. Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): None known. String or IDL/UUID changes made by this patch: None
Attachment #787806 -
Flags: approval-mozilla-beta?
Attachment #787806 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Updated•11 years ago
|
Attachment #787806 -
Flags: approval-mozilla-beta?
Attachment #787806 -
Flags: approval-mozilla-beta+
Attachment #787806 -
Flags: approval-mozilla-aurora?
Attachment #787806 -
Flags: approval-mozilla-aurora+
Comment 23•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7c668796e2d2 https://hg.mozilla.org/releases/mozilla-beta/rev/3b873fda98fc
Reporter | ||
Comment 24•11 years ago
|
||
Comment on attachment 787862 [details] [diff] [review] tests patch Review of attachment 787862 [details] [diff] [review]: ----------------------------------------------------------------- r=me, thank you
Attachment #787862 -
Flags: review?(surkov.alexander) → review+
Comment 25•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e4e8d9baaac6
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•