194553 - Add accesskeys to bookmark properties

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[piers]

Add accesskeys to bookmark properties

Comment 1

[piers]

Attachment: Add accesskeys to bookmark props

Comment 2

[:deanis74]

Comment on attachment 115274 [details] [diff] [review]
Add accesskeys to bookmark props

You should add accesskeys to the check boxes on the Notify tab.  They'll work,
they just don't display due to bug 68841.

Comment 3

[piers]

Attachment: Add accesskeys to bookmark props v2

Done, although i don't really see the point in adding checkbox accesskeys if
they
can't be used (you can't use them if you don't know what they are!!)

Comment 4

[:deanis74]

Comment on attachment 115279 [details] [diff] [review]
Add accesskeys to bookmark props v2

Looks fine.  r=me

We should be specifying accesskeys for check boxes and radio buttons now, so
that after bug 68841 is fixed we don't have to go through every dialog again to
add more accesskeys.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 115279 [details] [diff] [review]
Add accesskeys to bookmark props v2

Wouldn't it be better to use "D" for display an alert?

Comment 6

[:deanis74]

It's used for 'Description'.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Description is in a different tab.  Is it bad or not possible to reuse it in
that case?

Comment 8

[:deanis74]

Not currently, no.  See bug 143065.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 115279 [details] [diff] [review]
Add accesskeys to bookmark props v2

Rather than living with inferior accesskeys, I think we should try and get bug
143065 fixed.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Marking as dependent on 143065.  If we get near the end of 1.4beta and this
still isn't done, then we can reconsider and perhaps land this patch.  But
getting 143065 fixed would be much preferable.

Comment 11

[piers]

Dan, yeah, that would be ideal. But 143065 is nine months old with only 4 
comments - doesn't look too active :(

Comment 12

[Alfred Kayser]

See Bug 117026 for a patched bm-props.xul, merging all tabs together.

Comment 13

[Darin Fisher]

alfred: you mention in bug 117026 that this bug should be fixed now, but i don't
see any access keys being bound to XUL elements in that patch.  i also couldn't
navigate the bookmarks properties dialog via access keys.  am i missing something?

Comment 14

[Alfred Kayser]

Attachment: Patch to add the accesskeys to the XUL

This patch addes the accesskeys to the XUL part.
Note, the accesskeys definitions are allready in bm-props.dtd
Note furthermore that bug 143065 is not relevant for this bug anymore,
as due to bug 117026 the tabpanels are now removed.

Comment 15

[Darin Fisher]

Comment on attachment 132991 [details] [diff] [review]
Patch to add the accesskeys to the XUL

looks ok to me, but note: you should get module owner review as well.

Comment 16

[[e-mail]]

Did you mean to get darin's sr and my r?

Comment 17

[jag (Peter Annema)]

Comment on attachment 132991 [details] [diff] [review]
Patch to add the accesskeys to the XUL

>@@ -84,7 +88,7 @@
>         </columns>
>         <rows>
>           <row align="center">
>-            <label value="&when.label;" control="dayRange"/>
>+            <label value="&when.label;"  accesskey="&when.accesskey;" control="dayRange"/>

Extra space here.

sr=jag

Maybe if someone's bored they could change "duration" to "interval" in some
other bug.

Comment 18

[Alfred Kayser]

Attachment: Fixed the space

Comment 19

[Darin Fisher]

fixed on trunk for 1.7 alpha