221503 - an empty Personal Toolbar causes the browser window to continually vibrate

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Pete Boyd]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030925
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20030925

Mozilla doesn't install out-of-the-box on Windows the way I'd like it to behave
for the people I install it for. If I'm installing Mozilla for more than 2
people I want to automate the process of configuration; I expect a lot of people
are doing this so I think its important that pre-configuration works and is easy
to understand. I'm installing it for perhaps a hundred people, so I definately
need to automate the configuration.
Pre-configuration needs to be made using Mozilla's program-wide settings,
because thats what pre-configuration is all about and because from a Windows
batch script we can't make settings in users' profiles as their location is
unknowable due to the directory salting.
One part of the configuration I want to make is removing the 'Home' button from
the Personal Toolbar. To do this I insert this line at the bottom of
%PROGRAMFILES%\Mozilla\defaults\pref\all.js (using a batch script):
pref("browser.toolbars.showbutton.home", false);

But this only removes the tick in the box in the preferences dialog, it doesn't
actually stop the button from showing. To actually stop the button from showing
I have to edit %PROGRAMFILES%\Mozilla\defaults\profile\US\localstore.rdf.
In Mozilla 1.3 and 1.4 I had to add this to the bottom of localstore.rdf, before
the  '</RDF:RDF>':

  <RDF:Description about="chrome://navigator/content/navigator.xul">
    <NC:persist resource="chrome://navigator/content/navigator.xul#main-window"/>
    <NC:persist resource="chrome://navigator/content/navigator.xul#home-button"
                   hidden="true" />
  </RDF:Description>

(I put other sections in there too to remove other buttons but I'm just keeping
this report to the Home button for simplicity)

This method doesn't work in Mozilla 1.5RC2. In Mozilla 1.5RC2 the syntax appears
to have changed to:
  <RDF:Description about="chrome://navigator/content/navigator.xul#home-button"
                   hidden="true" />

(I don't understand RDF, I've just culled my understanding from reading the
files themselves; when I first learned to pre-configure Mozilla way back there
wasn't any documentation to read)

If I insert into localstore.rdf using this syntax instead then it still doesn't
remove the Home button.
So, how do I pre-configure this aspect of Mozilla in 1.5?
(this change doesn't appear to have been documented in Bugzilla)

Reproducible: Always

Steps to Reproduce:

Comment 1

[timeless]

.

Comment 2

[[e-mail]]

The following syntax is absolutely correct localstore RDF. IMHO anything else is
just plain wrong, and was never guaranteed to work.

<RDF:Description about="chrome://navigator/content/navigator.xul">
  <NC:persist resource="chrome://navigator/content/navigator.xul#home-button" />
</RDF:Description>
<RDF:Description about="chrome://navigator/content/navigator.xul#home-button"
                 hidden="true" />

Comment 3

[Pete Boyd]

Marvellous, that works, thanks Neil.

I take responsibility for the way I've gone about this, copying from existing
files without understanding the code, that if it works but shouldn't then it may
break in future.

As an aside, it would be useul if changes between Mozilla releases to all.js,
editor.js, localstore.rdf, panels.rdf etc could be documented for people who're
pre-configuring using them (i.e. there were additions to localstore.rdf in
Mozilla 1.4 compared with 1.3, for 'sidebar-box' and 'sidebar-splitter')

should I file another bug to the effect that 'browser.toolbars.showbutton.home'
and others like it cannot be used to actually remove buttons?

Comment 4

[timeless]

in the future you should consider swiping the localstore.rdf from a
profile and putting it into defaults (you can redact it first).

Comment 5

[Pete Boyd]

I don't know why I thought Neil's code worked, as it doesn't. It still makes the
window bounce up and down. I can only get the window to stay still if I use:

RDF:Description about="chrome://navigator/content/navigator.xul">
 <NC:persist resource="chrome://navigator/content/navigator.xul#home-button" />
 <NC:persist resource="chrome://navigator/content/navigator.xul#go-button" />
 <NC:persist resource="chrome://navigator/content/navigator.xul#bookmarks-button" />
/RDF:Description>
<RDF:Description about="chrome://navigator/content/navigator.xul#home-button"
                 hidden="true" />
</RDF:Description>
<RDF:Description about="chrome://navigator/content/navigator.xul#go-button"
                 hidden="true" />
</RDF:Description>
<RDF:Description about="chrome://navigator/content/navigator.xul#bookmarks-button"
                 hidden="true" />
</RDF:Description>

and then I'm still left with the Bookmarks button on the Personal Toolbar. Am I
missing something?

Comment 6

[Pierre Chanial]

goldenear: could you attach your the localstore.rdf to see it is well formed?
and btw, why don't you delete it before installing?

Comment 7

[Pete Boyd]

Attachment: localstore.rdf with Neil's recommended syntax

this is what I'm currently using and it causes the browser window to jump up
and down

Comment 8

[Pete Boyd]

"and btw, why don't you delete it before installing?"

delete it from Mozilla's program directory? I do
delete the user's version from their profile directory? I have a batch script
and I use that to wipe out the profile before testing

Comment 9

[[e-mail]]

Aha! Is this because when Mozilla opens a browser window without the bookmarks
or home buttons, there are no buttons at all on the personal toolbar, until the
bookmarks finish loading, when suddenly there are, thus changing the size of the
personal toolbar?

Comment 10

[Pete Boyd]

the problem is that the browser window bounces up and down continuously, until
its closed down.
I'm also wiping out the bookmarks using a pre-configured bookmarks.html, which
is stripped down and doesn't include anything on the Personal Toolbar.
if you want to check it out, the script is included in TWEAK which you can
checkout at http://thegoldenear.org/tweak/. but I've tried this with and without
my changes to bookmarks.html and nailed it down to when I only change
localstore.rdf. can anyone reproduce it using the attached localstore.rdf?

Comment 11

[[e-mail]]

OK, so there are two issues at work here:

1. The resizeFunc is called on content resize as well as chrome resize
2. The resizeFunc also needs to early return when there are no bookmarks

Patch coming up.

Comment 12

[[e-mail]]

Attachment: Proposed patch

Comment 13

[Jan Varga [:janv]]

Comment on attachment 132992 [details] [diff] [review]
Proposed patch

r=varga

Comment 14

[Pete Boyd]

Will this make it into 1.5?

Comment 15

[Alec Flett]

Comment on attachment 132992 [details] [diff] [review]
Proposed patch

sr=alecf

Comment 16

[[e-mail]]

Fix checked in to the trunk.

If you want this in 1.5 set approval1.5 to ? and make your case to drivers.

Comment 17

[Jo Hermans]

The bouncing that is mentioned in comment 10, is that the same as in bug 219643,
bug 221725 and bug 221779 ?

Comment 18

[Pierre Chanial]

Neil: good catch
Thought, this patch has two flaws:
- the first one: you bail in |resizeFunc| without unsetting the flag
|_overflowTimerInEffect|. That means that if you move out all the bookmarks in
the BT, then it will no more be possible to have the chevron back.
- then, more importantly, it breaks the reinitializing of the bookmarks toolbar
when its elements are reordered via the sidebar, the bm, etc...
Even if there is no argument in the setTimouts in the bookmarksToolbarMenu
observer, |event| is not null in |resizeFunc|, as you assumed.
That's because of the the "secret" argument that is added to function called via
the timer:
http://lxr.mozilla.org/seamonkey/source/dom/src/base/nsGlobalWindow.cpp#5003
You should simply force the null argument in the observer.

Comment 19

[Pierre Chanial]

forgot to reopen

Comment 20

[[e-mail]]

Attachment: Supplementary patch

pch, thanks for the lowdown on setTimeout

Comment 21

[[e-mail]]

Comment on attachment 133027 [details] [diff] [review]
Supplementary patch

OK, so hopefully this fixes the timer issues.

Comment 22

[Pierre Chanial]

Comment on attachment 133027 [details] [diff] [review]
Supplementary patch

I think you should unset |_overflowTimerinEffect| at the very beginning of
|resizeFunc|. If |bookmarks-ptf| is hidden, then a modification of the
resources in the personal toolbar will still prevent the chevron from working
if the user unhide it. (but that was a bug prexisting your patch)
with that r=me.

Comment 23

[Pierre Chanial]

err, my last comment is not correct, since the observer goes away with the
bookmarks toolbar. But I still think it's better practice.

Comment 24

[[e-mail]]

pch: well not right at the top, after the event check surely?

Comment 25

[Alec Flett]

Comment on attachment 133027 [details] [diff] [review]
Supplementary patch

sr=alecf

Comment 26

[Pierre Chanial]

It is safe to assume that only a resize event can bail in the event check.
But I was thinking that putting the line on top would prevent people from
putting additional checks before unsetting the flag but I don't have a so strong
opinion. Do what you want.

Comment 27

[[e-mail]]

Attachment: Patch that I actually checked in

Comment 28

[[e-mail]]

I hope I got it right this time :-[

Comment 29

[Pete Boyd]

WFM on recent trunk build :)

Comment 30

[Pete Boyd]

my localstore.rdf method of pre-configuration is now working as expected.

tho it may be obvious why this happens to those of you that understand the
source, I thought I'd mention it just in case its useful to know... in testing I
found that having configured the 1.6a build 2003101304 to have nothing on the
Personal Toolbar, then uninstall that version and install 1.5RC2, using the same
profile, 1.5RC2 had the empty Personal Toolbar that didn't bounce up and down,
until an item was manually added then removed from it

Comment 31

[[e-mail]]

Yes, I'd found that some operations start the bouncing and others stop it.

Comment 32

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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

Comment 34

[Aaron Slunt]

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