257237 - [FIXr]passwords not displayed in password manager (after show passwords)

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Andrew Schultz]

After opening the password manager and attempting to show the passwords, the
columns are changed to Site / Username / Password, but the passwords are not
actually displayed.  If I cover up part of the window and then uncover it, the
passwords show up in that part of the window.  With the classic theme, hovering
the "Close" button also fixes it, but that doesn't work in Modern.

This regressed between linux trunk 2004080905 and 2004081006.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Possible fixes in that range that could be responsible: bug 230170, bug 69355
(unlikely).  Chances are, it's the former.  Can you try backing out that patch
locally and seeing whether that helps?  Or just adding a call to
ProcessRestyles() right after the restyle event is posted in
nsCSSFrameConstructor::AttributeChanged?

Comment 2

[Andrew Schultz]

> Or just adding a call to ProcessRestyles() right after the restyle event is 
> posted in nsCSSFrameConstructor::AttributeChanged?

that fixed it.

Comment 3

[Boris Zbarsky [:bzbarsky]]

<sigh>.  I'll look into this when I have a tree again.  Any help in locating the
exact attribute change responsible would be much appreciated...

Comment 4

[Boris Zbarsky [:bzbarsky]]

OK, this is mine.

The short of it is that nsTreeColFrame::AttributeChanged invalidates the tree
for changes to the "hidden" attribute (as here).  The problem is that the actual
restyle (which changes the visibility and triggers the reflow) happens async; in
particular, it happens after the invalidate.

The problem, in short, is that changing an attribute on a node has to invalidate
some other node's frame.

I suppose I could keep track of this in the attr change data I post in the frame
constructor (probably flag these exact two attributes with a boolean in the
event I post for cases when the frame is a treecol frame and either call
AttributeChanged() after I process the restyles or call some other API that I
can add).

But is there any way to make this happier?  Shouldn't any reflow on the treecol
frame cause an invalidate of the tree?  Or are treecol frames not reflown?  I
don't see a Reflow() method on them....

The reason I think there should be a better way to do this is that it's possible
to show the treecol frame without changing the "hidden" attribute (eg by setting
the "visibility" property by some other CSS rule than the one in XUL.css).  This
is broken in the current code even without the patch to bug 230170.

Comment 5

[Boris Zbarsky [:bzbarsky]]

To be more precise, _someone_ is changing the mRect of the treecol frame. 
That's what should invalidate the tree, not magic attribute changes.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Also note that on nsIFrame SetRect/SetSize/etc are non-virtual, as is
Invalidate, so I can't easily hook into them to find who changes the size of the
treecol frame.  Here's hoping Neil or Jan knows.  If not, I guess I'll change
nsIFrame and start experimenting...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Some more investigation shows that the DoLayout() method of the treecol is
called, but that the Reflow()/DidReflow() methods are now.

I could probably just hook into SetBounds() for this invalidation business, I
guess...  Patch coming up, hopefully.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed fix

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 160233 [details] [diff] [review]
Proposed fix

roc, if we plan to make SetBounds non-virtual, then we'll need some other way
to notify XUL boxes that layout is done...

Comment 10

[Jan Varga [:janv]]

Comment on attachment 160233 [details] [diff] [review]
Proposed fix

yes, this is more correct

Comment 11

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk for 1.8a5.