Closed Bug 357027 Opened 18 years ago Closed 17 years ago

The 'New event' and 'New Task' windows have no accesskey

Categories

(Calendar :: General, defect)

All
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: cedric.corazza, Unassigned)

References

Details

(Keywords: access)

Attachments

(2 files, 2 obsolete files)

These windows have no accesskey. I expect to provide a patch before the end of this week.
As most labels are common in both the windows and the numbers of letters are limited, so I had to use some accesskeys that are not recommended ('i' for Title, 't' for Due date --New Event Windows-- and 'y' for Priority). At least, this is the best combination I found ans there is no accesskey collision. I also removed from the dtd file 3 entities that were not used.
Attachment #242666 - Flags: first-review?(cmtalbert)
Comment on attachment 242666 [details] [diff] [review] Patch adding accesskeys for tne New Event and New Task Windows R + with my comment about wrapping addressed. Here was our discussion about the patch: Hi Cedric, I have some comments <Cedric> yes ? <ctalbert> First is a general one, are you familiar with Mozilla's 80 character line rule? <ctalbert> (I ask because I wasn't) <Cedric> nope :") <ctalbert> It's ok. <Cedric> That, I can fix easily <ctalbert> The idea is that they want to limit lines to no more than 80 characters in order to help the readability of things, particularly in bugzilla diffs. <Cedric> understood <ctalbert> So, there's several lines that need to be adjusted there. <Cedric> ok. Something else to fix ? <ctalbert> yes, had to go look it up. <ctalbert> Down in calendar.dtd <Cedric> ok. I'm online for at least 3 hours <ctalbert> You removed ENTITY newevent.category.label <Cedric> yes <ctalbert> Why? <Cedric> it doesn't appear anywhere in the xul files <ctalbert> That would be a good reason. <ctalbert> Hmm...you're right. I didn't realize that. <ctalbert> Does the same hold true for the other items you removed? <Cedric> I guess that the task.category entity is used for that <ctalbert> I guess so. <ctalbert> Ok. I'm lxr searching the other entity names, and finding that they aren't used anymore either. <Cedric> yes : I made a grep -R to find these 3 entities all around the /calendar directory <ctalbert> Okay that's cool then. <ctalbert> Oh, a question I wanted to ask you so I can learn from you. <Cedric> calendar.dtd is quite messy : the entities are spread averywhere in this file and not gathered by dialog, as it used to be for other files <ctalbert> I can see that. It is a mess. <ctalbert> What purpose does the "control="item-title"> tag serve? <ctalbert> I'm not familiar with the "control" tag <Cedric> to put the focus on the right place <ctalbert> oh. I see. <ctalbert> Okay, so my only nit is that you just need to wrap those long lines. <Cedric> but this is not necessary for checkbox, etc... <Cedric> ok. I'll do that this evening <ctalbert> I'll post this log into a comment on the bug,and you'll have my r+ with those lines wrapped. Nice work. <Cedric> ok. thx
Attachment #242666 - Flags: first-review?(cmtalbert) → first-review+
Attached patch Fixing the 80 char. per line rule (obsolete) (deleted) — Splinter Review
This one fixes the 80 characters-per-line rule for patch. I didn't see this before. Maybe this rule worth to be documented, or, if already, highlighted.
Attachment #242666 - Attachment is obsolete: true
Attachment #242690 - Flags: second-review?(jminta)
Comment on attachment 242690 [details] [diff] [review] Fixing the 80 char. per line rule Yay accessibility! + <label value="&newtodo.duedate.label;" class="todo-only" accesskey="&newtodo.duedate.accesskey;" control="todo-duedate"/> This guy didn't get wrapped. r2=jminta with that. Thanks for the patch!
Attachment #242690 - Flags: second-review?(jminta) → second-review+
This patch fixes the wrapping issue raised by jminta. It should be good now
Attachment #242690 - Attachment is obsolete: true
Whiteboard: [checkin needed]
Patch checked in on MOZILLA_1_8_BRANCH and trunk. -> FIXED
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
There are wrong control attributes for the 'New task' dialog. Patch follows
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #243012 - Flags: first-review?
Comment on attachment 243012 [details] [diff] [review] [checked in] Fixing the wrong control attributes This moves control from the datepickers (which will be disabled until the checkbox is checked) to the checkbox. r=lilmatt
Attachment #243012 - Flags: second-review?(mvl)
Attachment #243012 - Flags: first-review?
Attachment #243012 - Flags: first-review+
Comment on attachment 243012 [details] [diff] [review] [checked in] Fixing the wrong control attributes r2=mvl
Attachment #243012 - Flags: second-review?(mvl) → second-review+
Patch checked in on MOZILLA_1_8_BRANCH and trunk. -> FIXED
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20061031 Calendar/0.4a1 I tried the accesskeys. They are there, but the some of them aren't working for me: Event: - To (T) -> nothing happens. Task: - date (a) -> nothing happens. - Status has no accesskey.
(In reply to comment #12) I have spent quite a bit debugging this because the patch looked very good to me when I reviewed it, and the checked in code looked good too. However, koen has a point, the following access keys do not work: --Tasks-- the "a" for the Task Entry Date The status field has no access key --Events-- The "F" for From Date does not work The "T" for To Date does not work. I looked into the Task issue specifically, since the "t" for Due Date _does_ work. I managed to verify that it is the actual choice of "a" that seems to be messing things up. If you change the xul at http://lxr.mozilla.org/seamonkey/source/calendar/base/content/calendar-event-dialog.xul#117 to be: <label value="&newevent.date.label;z" class="todo-only" accesskey="z" control="todo-has-entrydate"/> You will find that the "z" access key does work. I can't explain what is wrong with the "a" selection, but at least this proves that it is not a weird datepicker issue (which was my first thought), and that it is definitely related to the access keys. Therefore, sadly, I will reopen this bug.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The problem here is that the same dialog is used for both tasks and events. Therefore, when creating accesskeys for this dialog, you have to take into account which access keys are used on BOTH the "New Event" and "New Task" page. And you can only use the accesskeys that are NOT in use on either page. For example, the trick in comment 13 works because "a" is defined as an Access key for "All Day" on the "New Event" dialog (which is hidden on the "New Task" dialog). Similiarly, if you were to leave the "Date" accesskey as "a" on the Tasks page, and you changed the accesskey for "All Day" to something other than "A", then you would find that the "Date" item on the "New Task" dialog would begin working. The problem that you will quickly run into is that you may run out of available letters for your accesskeys since each accesskey can only be defined once on both the "New Event" and "New Task" dialog. Perhaps that can be addressed by providing accesskeys for more commonly used dialog features, and leaving them off of the more uncommon dialog features?
Depends on: 143065
Is this still an issue with the new event/task dialog (AKA prototype dialog) in current 0.8pre nightly builds? If yes I would prefer to close this bug that belongs to the old dialog and file a new bug for the new dialog.
Though bug 143065 has been fixed, the behavior stated in comments 12 to 14 still occurs : ALT+s and ALT+n don't focus on Start and End. I guess this could be part of a follow-up bug. Whatever, I'm not skilled to fix this. Verified with : Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13pre) Gecko/20080218 Calendar/0.8pre
Attachment #242806 - Attachment description: Fixing the wrapping issue → [checked in] Fixing the wrapping issue
Attachment #243012 - Attachment description: Fixing the wrong control attributes → [checked in] Fixing the wrong control attributes
(In reply to comment #16) > Whatever, I'm not skilled to fix this. -> Reassigning to nobody.
Assignee: cedric.corazza → nobody
Status: REOPENED → NEW
The dialog in question is no longer available. I see no reason to keep this bug opened.
Marking WONTFIX, as this bug is targeted at the old event dialog. Please file outstanding issues in the new dialog in followup bugs!
Status: NEW → RESOLVED
Closed: 18 years ago17 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: