68662 - Browser UI doesn't update after skin switch

Status: VERIFIED FIXED

(Core Graveyard :: Skinability, defect)

Description

[Peter "jag" Annema]

After switching skins, the browser UI no longer updates.

This bug was introduced with the checkin to nsBrowserInstance.cpp on Jan 4 which
refactored some of the code in there, by no longer reinitializing the content
variables, amongst which registering the web progress listener with the new
content window's docshell, if the old content window had gone away.

I've got a fix, now I just need to write it :-)

Comment 1

[:deanis74]

Could this be what's messing up menus after a switch as well?

Comment 2

[R.K.Aa.]

in which case it's bug 67574

Comment 3

[Peter "jag" Annema]

No, that's another, unrelated, bug.

Comment 4

[Brendan Eich [:brendan]]

jag, should we get this into 0.8 at the last minute?

/be

Comment 5

[Peter "jag" Annema]

Well, this turns out to be a bit trickier than I thought it was.

I've got something which works like things worked before my changes, but it
turns out that even before those changes, clicking links after switching themes
would not update the browser UI. The listener object (BrowserInstance) needs to
be registered with the new content docshell, and that doesn't happen for link
clicking. I'm investigating an ugly hack at the moment which could fix that too.

Comment 6

[Peter "jag" Annema]

Nah, the hack is too ugly for 0.9 and too big a risk for 0.8. The work-around is
to open a new browser window and close the current one, or close Mozilla and
start again.

I'll work on a better fix for 0.9.

Comment 7

[Fabian Guisset]

Jag, you may find these bugs interesting : 61796, 61991, 68230, 68227.

Comment 8

[Blake Ross]

-> skinability

Comment 9

[Blake Ross]

Didn't mean to reassign owners...

Comment 10

[Peter "jag" Annema]

Attachment: [patch] Moving garbage around

Comment 11

[Peter "jag" Annema]

Attachment: [patch] Also move re-hooking up the secure browser UI object

Comment 12

[Alec Flett]

cool... I like this way of solving it. :)
sr=alecf

Comment 13

[Ben Goodger (use ben at mozilla dot org for email)]

jag tells me that this is an intermediate stage in browser cleanup work, so an 
r=[e-mail] for this change. 

Comment 14

[djk]

I noticed UI breakage with 2001031008 on win98se after applying a theme.
The back button's dropdown menu didn't work and the address bar stopped updating
itself as I surfed.

It looks like this bug has an r and sr...when's it gonna be checked in?

Comment 15

[Peter "jag" Annema]

As soon as I find out why I can't get the SecureBrowserUI component to get
hooked up again (it's what sets the little lock icon corresponding to the page
you're viewing). It should all theoretically work, but apparantly our browser
thinks differently ;-)

I'll need to get a build which doesn't crash on me to get some progress there,
though I may land this patch as-is for 0.8.1 and work from there.

alecf, what say you?

Comment 16

[Asa Dotzler [:asa]]

I'd like to see this go in for 0.8.1 
a=asa on behalf of drivers. Not sure what's happening with the tree yet but when
we are ready to start taking checkins let's get this in.

Comment 17

[Peter "jag" Annema]

Attachment: [patch] This one actually works... messenger worries me though (new bug?)

Comment 18

[Alec Flett]

discussed with jag on irc...sr=alecf

Comment 19

[Peter "jag" Annema]

Checked in, marking fixed.

Comment 20

[Stephen P. Morse]

I'm not sure how this bug differs from bug 60151 et al (i.e., 61796, 61853, 
61991, 68227, 68230, etc.).  Those bugs deal with individual items in the UI 
which are not being updated and they are still not fixed, so how can this one be 
marked fixed?

Comment 21

[Peter "jag" Annema]

Steve and I found out that this was actually a regression caused by my checkin
on 2001-04-14 for bug 46200. Leaving this bug fixed, dealing with that issue in
bug 60151.

Comment 22

[Blake Ross]

Patty, can you see how this is working now on Linux and Mac?

Comment 23

[Patty Mac]

Blake, sorry for responding late. Anyway, I can not verify this bug
on Mac, Linux, and windows (2001-05-17-07-Mtrunk) because the classic
theme is broken due to switching themes. Will verify the later builds
though.

Comment 24

[Patty Mac]

This one is already fixed. Marking verified on all platforms
(commercial build: 2001-06-01-08-trunk).