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.