18859 - [DOGFOOD] URL bar messed up

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Colin Blake]

I originally raised this in n.p.m.seamonkey/unix. People seems to think that its
related to other problems that folks are working on. Since this problem did NOT
exist in M10 (only in M11), I am willing to believe this. Hopefully this problem
will go away by itself as other bugs are fixed. But since this is a major
usability issue for OpenVMS, I am entering this bug so that I don't forget about
it.

> > On OpenVMS, sometimes when I type into the URL bar, it gets confused.
> > But not always. Sometimes it'll work fine, but once the problem starts,
> > the only way to stop it is to exit and restart Mozilla. This is with the
> > M11 code (never saw the problem on M10). Here's what happens.
> >
> > Say I'm at xyz.com and so the URL bar displays:
> >
> >   http://xyz.com
> >
> > If I then click in the URL bar, and delete all the characters there, and
> > then type "xyz.com", I will be taken to xyz.com (same page as before)
> > but the URL bar will say:
> >
> >   http://xyz.com/xyz.com
> >
> > The vertical I-bar is left between the last "/" and the "x".
> >
> > If I now clear the URL bar again and type "fred", the URL bar displays:
> >
> >   http://fred/fred
> >
> > which doesn't load, and when I stop it, the message in the console
> > window says:
> >
> >   Error loading URL http://fred/
> >
> > so its only the display in the URL bar that's wrong.

Comment 1

[Colin Blake]

Possible causes are:

> Could this be related to the known problems with painting text fields in
> gtk, 16266, 13407, and 17348?  Some of these bugs do involve text fields
> that have content but aren't being painted, or at least, that's
> something I've seen that I assume is covered by one or more of these
> bugs.

Comment 2

[tor]

I'm seeing this on solaris/native as well.  It's not just a painting problem -
the text is really in the entry area - the cursor can be placed in the extra
text and it can be edited like normal text.

The url bar doesn't always behave like this, but once it gets into this mode,
the text never gets deleted and new urls just get appended to what is already
there.  Double-clicking selects all the text, but typing doesn't delete the
selection.

When you click on a URL and the url bar is misbehaving, the following console
message is printed:

PostCondition: "transaction did not execute properly
" ((NS_SUCCEEDED(result))) at file
/cs/src/mozilla/mozilla/editor/base/nsEditor.cpp, line 408
Break: at file /cs/src/mozilla/mozilla/editor/base/nsEditor.cpp, line 408

Maybe someone on the editor team should take a look at this?

Comment 3

[tor]

Adding kin and pavlov to CC list - you were suggested as people who might
be able to give useful comments on this bug.

Comment 4

[kinmoz]

From the descriptions above, it looks like the SelectAll-Delete that happens
when you replace the contents of the Gfx textwidget is failing.

Cc'ing [e-mail] to see if he has any clue on this. Line 408 of
nsEditor.cpp is in nsEditor::Do().

Comment 5

[kinmoz]

Reassigning to [e-mail].

I can't recreate this problem, but I showed [e-mail] how to turn logging
on during InsertText() calls so we can see what the selection is whenever it
fails. He should be updating the bug with the log output when it happens to him
again.

I think that the problem is in nsGfxTextControlFrame::SelectAllTextContent().
There's some suspicious code in there that will only set the end of the
selection if the last child in the body is a text node, otherwise it just leaves
it sitting at the first child. I think what we need to do is have the
GfxTextControlFrame code use nsIEditor::SelectAll() instead of rolling it's own.

Comment 6

[kinmoz]

Accepting bug.

Comment 7

[tor]

Ok, here we go:

selRanges = [ [ [[0, 1, 0],   0], [[0, 1, 1],   0] ] ];
EditorSetSelectionFromOffsets(selRanges);
window.editorShell.InsertText("http\u003a\u002f\u002fwww\u002eopengl\u002eorg\u002f");
PostCondition: "transaction did not execute properly
" ((NS_SUCCEEDED(result))) at file
/cs/src/mozilla/mozilla/editor/base/nsEditor.cpp, line 408

Comment 8

[kinmoz]

I'll bet that the 2nd node is either an empty text node from a previous call to
replace the text or our bogus BR node.

Comment 9

[leger]

Unless this is reproducable on Linux, not for dogfood. Putting on the PDT-
radar.

Comment 10

[kinmoz]

colin or tor, I haven't been able to reproduce the problem. Could you both try
this patch and see if it fixes the problem? This patch modifies:

    mozilla/html/forms/src/nsGfxTextControlFrame.cpp

so that it uses the editor's nsIEditor::SelectAll() method instead of it's own
internal SelectAllText() method.


Index: nsGfxTextControlFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/forms/src/nsGfxTextControlFrame.cpp,v
retrieving revision 3.96
diff -r3.96 nsGfxTextControlFrame.cpp
912,916d911
<       nsCOMPtr<nsIDOMSelection>selection;
<       result = mEditor->GetSelection(getter_AddRefs(selection));
<                       if (NS_FAILED(result)) return;
<                       if (!selection) return;
<
922,925c917
<       nsCOMPtr<nsIDOMNode> bodyNode;
<       nsAutoString bodyTag = "body";
<       result = GetFirstNodeOfType(bodyTag, domDoc, getter_AddRefs(bodyNode));
<       SelectAllTextContent(bodyNode, selection);
---
>       result = mEditor->SelectAll();
2383c2375
<       result = SelectAllTextContent(bodyNode, selection);
---
>       result = mEditor->SelectAll();

Comment 11

[tor]

Haven't been able to reproduce the problem with this patch in my tree.

Comment 12

[Colin Blake]

I just got my M12 code up and running, and the problem does NOT occur in the M12
code, even though I haven't applied the patch yet.

Until I can reproduce the problem in M12, I won't apply the patch. I'll keep
testing, and will post my findings back here.

Comment 13

[Colin Blake]

I applied the patch to my M11 code tree, and I can no longer reproduce this
problem. Looks like a fix to me!

Comment 14

[Joe Francis]

I have already checked in Kin's fixes for this.  It should be fixed.

Comment 15

[buster]

This fix also fixes bug 14803.  dup-ing that bug to this.  I will remove the
dead code from text control frame.

Comment 16

[buster]

*** Bug 14803 has been marked as a duplicate of this bug. ***

Comment 17

[Colin Blake]

This is so fixed. If someone will close, I'll verify, and this bug will be
history.