Steps to reproduce: 1. pref devtools.chrome.enabled = true 2. open Scratchpad 3. menu Environment -> Browser 4. click the X to dismiss the warning 5. Cmd+A or Ctrl+A to select the contents of the scratchpad 6. hit Delete/Backspace Expected: the contents of the scratchpad should be deleted in step 6 Observed: everything works fine through step 5, but on hitting Delete in step 6, nothing happens

The close button in the toolkit widget notification doorhanger was stealing focus from the scratchpad editor. "4. click the X to dismiss the warning" I added an event to be fired in when its close button is clicked, then listened for it in the scratchpad and set the focus back accordingly. This tests out on my local WIN machine.

Comment on attachment 646955 [details] [diff] [review] Patch (v1) The notification not giving back focus is a bug that should be fixed entirely within notification.xml.

In that case, this works also. I built the original patch, as I'm also looking at Bug 609930 - Add AlertClosed event to notification boxes, and thought to make use of the new event.

I imagine "tabbable" was added for a reason (keyboard accessibility?). Have you checked hg blame?

I wondered about a11y, having done a buncha bugs for them, and this did seem to imply effects in general outside the scratchpad might occur. The first patch fixes both (I believe) but there might be a better / best way I'm missing yet.

I guess I didn't answer your question. HG blame command line shows the line revise number is 1, but then I found on github blame Bug 347247 - No way to dismiss a notification without a mouse ... Must be for notificationboxs in general... in Scratchpad I tried and couldn't find a way to tab to the close field while decided if this was a good solution for this bug ...

yeah, I can't review this in toolkit. One of the toolkit peers should take a look. Like gavin suggests, I would imagine "tabbable" was added for a11y reasons (haven't tested, can you tab out of the close widget on the notification box?). I don't feel comfortable making this change for the scratchpad without another reviewer.

Comment on attachment 647045 [details] [diff] [review] Patch (v2) Cancelling review, needs a toolkit peer.

Comment on attachment 647045 [details] [diff] [review] Patch (v2) Asking feedback request from the Toolkit/Widget peer name I've seen in IRC. (And summarizing) So Patch (v2) won't be correct due to the history I found in comment 6 showing that there's an a11y reason to leave the tabbable parm in place on the close button in the notification box. That leaves me the original Patch (v1) as a workable option with most changes on the ScratchPad side, while also completing an open Bug 609930 - Add AlertClosed event to notification boxes on the XP Toolkit/Widgets: XUL side for the moment.

Comment on attachment 647045 [details] [diff] [review] Patch (v2) The button needs to be focusable for accessibility.

I tested this with the scratchpad and in the tabbed browser, which suffers from the same problem.

Thank you! I was hoping you might agree to take it :)

Comment on attachment 647303 [details] [diff] [review] patch I assume this is caused by bug 570835?

(In reply to Neil Deakin from comment #13) > Comment on attachment 647303 [details] [diff] [review] > patch > > I assume this is caused by bug 570835? Yes, I'll add a comment with a reference to that bug.