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)
Tracking
()
VERIFIED
FIXED
M14
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.
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.
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?
Comment 4•25 years ago
|
||
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.
Comment 6•25 years ago
|
||
nsIControllersOwner?
Assignee | ||
Comment 7•25 years ago
|
||
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.
Comment 8•25 years ago
|
||
How about if nsIControllersOwner just had a "SetControllers()" method, so I could do the hookup?
Comment 9•25 years ago
|
||
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)
Assignee | ||
Comment 10•25 years ago
|
||
Which hookup are we talking about here?
Comment 11•25 years ago
|
||
Setting the command dispatcher in the controllers object when the controllers object is first created.
Assignee | ||
Comment 12•25 years ago
|
||
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
Reporter | ||
Comment 13•25 years ago
|
||
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.
Updated•25 years ago
|
Assignee: waterson → hyatt
Comment 14•25 years ago
|
||
hyatt: it's all you.
Reporter | ||
Comment 15•25 years ago
|
||
adding radha to the cc-list, since her work in the URL bar of the browser is dependent on this.
Comment 16•25 years ago
|
||
cc: self, since command updating in editor is also impacted.
Assignee | ||
Updated•25 years ago
|
Status: NEW → ASSIGNED
Target Milestone: M13 → M14
Assignee | ||
Comment 17•25 years ago
|
||
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.
Assignee | ||
Comment 18•25 years ago
|
||
*** Bug 21376 has been marked as a duplicate of this bug. ***
Assignee | ||
Updated•25 years ago
|
Whiteboard: Awaiting saari's command dispatcher move.
Comment 19•25 years ago
|
||
putting on beta1 radar
Comment 22•25 years ago
|
||
Putting on PDT+ radar for beta1.
Whiteboard: Awaiting saari's command dispatcher move. → [PDT+]Awaiting saari's command dispatcher move.
Assignee | ||
Updated•25 years ago
|
Whiteboard: [PDT+]Awaiting saari's command dispatcher move. → [PDT+]Fix in hand. Waiting for tree to open.
Assignee | ||
Comment 23•25 years ago
|
||
Fixed.
Assignee | ||
Comment 24•25 years ago
|
||
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Comment 25•25 years ago
|
||
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.
Reporter | ||
Comment 26•25 years ago
|
||
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.
Comment 27•25 years ago
|
||
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
Comment 28•25 years ago
|
||
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.
Description
•