377678 - toolbar.xml cleanup

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Neil Deakin]

- toolbarspacer and toolbarspring should have XULToolbarSeparator accessibility objects
- currentSet setter doesn't return a value
- insertItem doesn't work with no surrounding toolbox

Comment 1

[Neil Deakin]

Attachment: cleanup

Comment 2

[Neil Deakin]

Attachment: also fix up accessibility references

confirmed with aaronlev that only Toolbar should use nsIAccessibleProvider.XULToolbar and that the three spacing types should use nsIAccessibleProvider.XULToolbarSeparator.

Comment 3

[[e-mail]]

Comment on attachment 261956 [details] [diff] [review]
also fix up accessibility references

>+            if (this.localName == "toolbarseparator" ||
>+                this.localName == "toolbarspacer" ||
>+                this.localName == "toolbarspring")
>               return Components.interfaces.nsIAccessibleProvider.XULToolbarSeparator;
>             else
>               return Components.interfaces.nsIAccessibleProvider.XULToolbar;
Wouldn't it be better to override this in the toolbar binding?
(The menubar binding already does, for instance.)

>             if (val == "__empty")
>-              return;
>+              return val;
> 
>             if (val) {
if (val && val != "__empty") perhaps?
(That __empty setting smells like a cheap hack!)

Comment 4

[Neil Deakin]

Attachment: more like this

Comment 5

[[e-mail]]

Comment on attachment 262010 [details] [diff] [review]
more like this

>-    <implementation implements="nsIAccessibleProvider">
>     <implementation>
>+      <property name="accessibleType" readonly="true">
r=me with this fixed as mentioned on IRC.