Open Bug 1091187 Opened 8 years ago Updated 1 year ago

Move selectionchange code in nsGlobalWindow::UpdateCommands

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

People

(Reporter: enndeakin, Unassigned, Mentored)

Details

(Keywords: good-first-bug)

Attachments

(1 file, 1 obsolete file)

Bug 987040 made an unnecessary api change here and added code that doesn't have anything to do with updating commands. This should be undone and the code moved elsewhere.
Whiteboard: [good first bug]
Hi, I'd like to take this on as my first bug assuming its still valid.

From what I can see the affected code is here: 
https://dxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#9754

Can you give an explanation of the intended function of this code and the intended function of the code to be moved?
Flags: needinfo?(enndeakin)
Hi Jamie, thanks for taking this on. Some of the work for this has already been done in another bug. The remaining work is fairly simple:

1. Remove the two unused arguments from nsGlobalWindow::UpdateCommands.
2. Remove the check that looks for "selectionchange" within this method.
Mentor: enndeakin
Flags: needinfo?(enndeakin)
Attachment #8674765 - Flags: review?(enndeakin)
Comment on attachment 8674765 [details] [diff] [review]
v1 Remove checks from UpdateCommands

>diff --git a/dom/interfaces/base/nsIDOMWindow.idl b/dom/interfaces/base/nsIDOMWindow.idl
...
> 
>   // XXX Should this be in nsIDOMChromeWindow?
>-  void                      updateCommands(in DOMString action,
>-                                           [optional] in nsISelection sel,
>-                                           [optional] in short reason);
>+  void                      updateCommands(in DOMString action);

You'll need to change the uuid on the interface when the interface is changed. See https://developer.mozilla.org/en-US/docs/Generating_GUIDs

Otherwise this looks good.
Attachment #8674765 - Flags: review?(enndeakin) → review+
Assignee: nobody → flintj3
Status: NEW → ASSIGNED
Updated UUID in idl file
Attachment #8674765 - Attachment is obsolete: true
Flags: needinfo?(enndeakin)
Attachment #8675579 - Flags: review?(enndeakin)
Comment on attachment 8675579 [details] [diff] [review]
dom-updatecommands-v1.patch

OK!
Flags: needinfo?(enndeakin)
Attachment #8675579 - Flags: review?(enndeakin) → review+
Was this patch ready to land? Looks like this got stalled after being r+'d.
Flags: needinfo?(enndeakin)
Yes, it should be ok, but we likely will need an updated patch.
Flags: needinfo?(enndeakin)
Component: DOM → DOM: Core & HTML
Keywords: good-first-bug
Whiteboard: [good first bug]

This good-first-bug hasn't had any activity for 6 months, it is automatically unassigned.
For more information, please visit auto_nag documentation.

Assignee: flintj3 → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.