Closed Bug 230219 Opened 21 years ago Closed 20 years ago

go button is set off even when set on in prefs in installer

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.7final

People

(Reporter: matousek, Assigned: neil)

References

Details

(Keywords: fixed1.7)

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; cs-CZ; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; cs-CZ; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6

when customizing mozilla installer, if preference browser.toolbars.showbutton.go
is set on in the preference file all.js, when install this modified inslaller,
the go-button is hidden. I know i can edit navigator.xul too and set
"go-button"'s attribute hidden to false to show it, but i think it will be
better to do this only through prefs and do not edit xul files at all.

Reproducible: Always

Steps to Reproduce:
1. customize installer - set the browser.toolbars.showbutton.go pref to true
2. install this installer

Actual Results:  
the go-button is off

Expected Results:  
the go-button should be on
of course, this is about mozilla installer, not firebird instaler as the
UserAgent header look like. sorry :)
Summary: go button is set off ever when set on in prefs in installer → go button is set off even when set on in prefs in installer
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

Always assuming that this is what we should be doing
Attachment #138531 - Flags: review?(dean_tessman)
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

Why don't the other pref listeners need to be initialized like this?
Well, an obvious example, the home button tooltip is already initialized...
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

Is this still relevant?  If so, r=me.
Attachment #138531 - Flags: review?(dean_tessman) → review+
Attachment #138531 - Flags: superreview?(alecf)
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

are these listeners ever removed? Do we really need to set listeners on ever
pref in the domain? I thought the way pref listeners worked, you could just
listen to the toplevel domain?

i.e. this.observe(pref, "nsPref:changed", this.domain);
I'm not adding or removing listeners, I'm just poking the existing listener so
that it sees the initial value.
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

oh, duh. I see. I think I even wrote the original code :)

You're forcing the observe() to be called, when it would normally be called by
a pref change

can you put a comment to that affect in init()? something like "Force observe()
to be called so that we get the initial value of the pref on startup"

(I realize your comment "Ensure button visibility.." hints at this, but
still..)

sr=alecf with the comment
Attachment #138531 - Flags: superreview?(alecf) → superreview+
Target Milestone: --- → mozilla1.8alpha
bug 211829 dup of this?
Blocks: 211829
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

this is a pretty minor change and sure would be nice to get in for 1.7
Attachment #138531 - Flags: approval1.7?
Comment on attachment 138531 [details] [diff] [review]
(Av1) <navigator.js> Proposed patch
[Checked in: Comment 13]

a=chofmann for 1.7
Attachment #138531 - Flags: approval1.7? → approval1.7+
Fix checked in.
Status: UNCONFIRMED → RESOLVED
Closed: 20 years ago
Keywords: fixed1.7
Resolution: --- → FIXED
so to what extent did this fix bug 166455?
OS: Windows XP → All
Hardware: PC → All
Target Milestone: mozilla1.8alpha → mozilla1.7final
Attachment #138531 - Attachment description: Proposed patch → (Av1) <navigator.js> Proposed patch [Checked in: Comment 13]
Attachment #138531 - Attachment is obsolete: true
Adds:

1)
(In reply to comment #9)
> can you put a comment to that affect in init()? something like "Force
observe()
> to be called so that we get the initial value of the pref on startup"

2)
(In reply to bug 166455 comment 14)
> In fact lines 93-105 of navigator.js could probably be cleaned up too.
Attachment #145838 - Attachment description: (Bv1) <navigator.js> additional → (Bv1) <navigator.js> additional comment and cleanup
Attachment #145838 - Flags: review?(neil.parkwaycc.co.uk)
hmm.. This checkin was responsible for about 0.5% regression in Ts on both comet
and luna.  Anything we can do to mitigate that?
Hmm... it might be because the allLeftButtonsAreHidden() is getting called five
times; I'll have to try special-casing the startup code to fix that.
Status: RESOLVED → UNCONFIRMED
Resolution: FIXED → ---
Assignee: guifeatures → neil.parkwaycc.co.uk
Attachment #145838 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Comment on attachment 145901 [details] [diff] [review]
(Cv1) <navigator.js> Startup optimization

As well as separating out the button and separator update I used .hidden
because the startup check makes persist unnecessary (see attachment 145839 [details] [diff] [review]).
Attachment #145901 - Flags: superreview?(alecf)
Attachment #145901 - Flags: review?(varga)
Comment on attachment 145902 [details] [diff] [review]
(Cv1b) <navigator.js> Extra bulletproofing
[Checked in: Comment 26]

...is needed for bug 230219
Attachment #145902 - Flags: superreview?(alecf)
Attachment #145902 - Flags: review?(varga)
Attachment #145901 - Flags: superreview?(alecf)
Attachment #145901 - Flags: superreview?
Attachment #145901 - Flags: review?(varga)
Attachment #145901 - Flags: review?
Attachment 145902 [details] [diff] should also go into 1.7 - nominating as blocker
(Or there will be a lot of reports like bug 240288)
Flags: blocking1.7?
Attachment #145901 - Attachment description: Startup optimization → (Cv1) Startup optimization
Attachment #145901 - Flags: superreview?
Attachment #145901 - Flags: review?
Attachment #145901 - Attachment description: (Cv1) Startup optimization → (Cv1)<navigator.js> Startup optimization
Attachment #145901 - Attachment description: (Cv1)<navigator.js> Startup optimization → (Cv1) <navigator.js> Startup optimization
Attachment #145838 - Flags: review?(neil.parkwaycc.co.uk)
Blocks: 240339
The builds of Mozilla with this fix checked in have been unusable since 4/10.
Can they please be removed?
Flags: blocking1.7? → blocking1.7+
Comment on attachment 145902 [details] [diff] [review]
(Cv1b) <navigator.js> Extra bulletproofing
[Checked in: Comment 26]

sr=dveditz
this works
Attachment #145902 - Flags: superreview?(alecf) → superreview+
Attachment #145902 - Flags: review?(varga) → review+
Comment on attachment 145902 [details] [diff] [review]
(Cv1b) <navigator.js> Extra bulletproofing
[Checked in: Comment 26]

a=asa (on behalf of drivers) for checkin to 1.7
Attachment #145902 - Flags: approval1.7+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago20 years ago
Flags: blocking1.7+ → blocking1.7?
Keywords: fixed1.7
Resolution: --- → FIXED
Attachment #145902 - Attachment description: Extra bulletproofing → Extra bulletproofing [Checked in: Comment 26]
Attachment #145902 - Attachment is obsolete: true
Flags: blocking1.7?
Attachment #145902 - Attachment description: Extra bulletproofing [Checked in: Comment 26] → (Cv1b) <navigator.js> Extra bulletproofing [Checked in: Comment 26]
No longer blocks: 211829
*** Bug 211829 has been marked as a duplicate of this bug. ***
Product: Core → Mozilla Application Suite
Attachment #138531 - Attachment is obsolete: false
Attachment #145902 - Attachment is obsolete: false
Component: XP Apps: GUI Features → UI Design
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: