Closed Bug 20471 Opened 25 years ago Closed 25 years ago

need to update commands whenever frame state changes in text control

Categories

(Core :: Layout: Form Controls, defect, P3)

x86
Windows NT
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: buster, Assigned: hyatt)

References

Details

(Whiteboard: [PDT+]Fix checked in. Waiting for testcase feedback from engineer.)

I already have the entry points in place, just need access to the command dispatcher. that's bug 20470.
Depends on: 2253
Status: NEW → ASSIGNED
Depends on: 20470
No longer depends on: 2253
Target Milestone: M14
without this fix, some menu items will only enable/disable when you do focus changes, instead of dynamically whenever the user interacts with the text field. I don't think this is dogfood, so setting for M14. It is a beta blocker.
QA Contact update.
playing with waterson's fix for 20470, I added this code (condensed here): // get the "controllers" object from the text control if (controllers) { nsCOMPtr<nsIDOMXULCommandDispatcher> commandDispatcher; result = controllers->GetCommandDispatcher(getter_AddRefs(commandDispatcher)); if (NS_FAILED(result)) { return result; } if (commandDispatcher) { nsAutoString commandString("furby"); result = commandDispatcher->UpdateCommands(commandString); } } but the controllers from the text controls have no command dispatcher associated with them. Who is responsible for doing this initial hookup: calling SetCommandDispatcher on the controllers of text controls in both chrome and html content?
Do HTML edit fields implement nsIControllers? If so, I can eagerly do a setup on any HTML element that implements the interface. That's probably the piece that's missing.
sorta. the content node for <textarea> and <input type=text> implement nsIDOMNSHTMLTextAreaElement and nsIDOMNSHTMLInputElement respectively. These interfaces each have a GetControllers() method. There's a comment in nsXULCommandDispatcher.cpp that suggests this should be more generic... nsXULCommandDispatcher::GetControllers(nsIControllers** aResult) { //XXX: we should fix this so there's a generic interface that // describes controllers, // so this code would have no special knowledge of what object // might have controllers. So we can QI each content node for these interfaces to get controllers if available, and set the dispatcher on any that we find. But it sucks to have to QI for more than one interface.
nsIControllersOwner?
This would be the correct way to do it... make the content nodes implement a secret interface that would give me another way of getting to the controllers from the command dispatcher. You still want the GetControllers methods on the content node interfaces themselves however for scriptability purposes.
How about if nsIControllersOwner just had a "SetControllers()" method, so I could do the hookup?
Oh duh. I am an idiot. Ignore what I've been saying about "pushing" the controllers into the HTML element. buster: what you need to do is set up the command dispatcher in nsHTMLInputElement's GetControllers() method. You'll need to: 1. QI() the element's document for nsIDOMXULDocument, 2. call GetCommandDispatcher() on that 3. call SetCommandDispatcher() on your new controllers object. See http://lxr.mozilla.org/mozilla/source/rdf/content/src/nsXULElement.cpp#3321 for code that does this. Copy n' paste, baby. (Yeah, makes layout depend on XUL, so you might want to #ifdef INCLUDE_XUL this. Or, like hyatt said, make the super-secret interface, but that seems like overkill)
Which hookup are we talking about here?
Setting the command dispatcher in the controllers object when the controllers object is first created.
Houston, we have a problem. His text field doesn't occur in a XUL doc necessarily. It could be nested 26 levels deep inside an HTML doc buried in framesets. The nsIControllers implementation should use the nsPIDOMWindow interface to walk up to the topmost window. It should get the command dispatcher off the XUL doc in the outermost window and then set it on itself. All "nsIControllers"s should just do this by default. Another bug is that all XUL docs get a command dispatcher when only the topmost XUL doc should have one. That's one reason i was thinking the command dispatcher should maybe be on the window instead.
Assignee: buster → waterson
Status: ASSIGNED → NEW
Target Milestone: M14 → M13
This sounds like a bug that waterson or hyatt should own, at least through the part where the controllers are hooked up to the command dispatcher. Once that's done, the editor code is already in place to take advantage of it. See nsGfxTextControlFrame::InternalContentChanged() Guessing M13.
Assignee: waterson → hyatt
hyatt: it's all you.
adding radha to the cc-list, since her work in the URL bar of the browser is dependent on this.
cc: self, since command updating in editor is also impacted.
Status: NEW → ASSIGNED
Target Milestone: M13 → M14
I can't work on this until saari finishes moving the command dispatcher. Since that hasn't happened yet, I'm going to have to push this off to M14.
*** Bug 21376 has been marked as a duplicate of this bug. ***
Whiteboard: Awaiting saari's command dispatcher move.
putting on beta1 radar
Updating severity to critical.
Severity: normal → critical
Adding beta1 keyword
Keywords: beta1
Putting on PDT+ radar for beta1.
Whiteboard: Awaiting saari's command dispatcher move. → [PDT+]Awaiting saari's command dispatcher move.
Whiteboard: [PDT+]Awaiting saari's command dispatcher move. → [PDT+]Fix in hand. Waiting for tree to open.
Fixed.
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Hey Dave, How do we test this? Is there a specific testcase where this would manifest itself? Thanks dude, -Kritzer
Whiteboard: [PDT+]Fix in hand. Waiting for tree to open. → [PDT+]Fix checked in. Waiting for testcase feedback from engineer.
test case is easy. open mozilla. click in url bar. open edit menu. notice what is enabled and disabled. for example, undo should be disabled since you haven't changed anything in the url bar. click back in the url bar. delete a character open the edit menu again. undo should be enabled now, because the text control has changed state.
Marking VERIFIED FIXED on: - Linux6 2000-02-17-08 Commercial build - MacOS9 2000-02-16-16 Mozilla build - Win98 2000-02-16-16 Commercial build
Status: RESOLVED → VERIFIED
Marking VERIFIED FIXED on: - Linux6 2000-02-17-08 Commercial build - MacOS9 2000-02-16-16 Mozilla build - Win98 2000-02-16-16 Commercial build
You need to log in before you can comment on or make changes to this bug.