43714 - Tooltip for the 'Home' button not updated when setting page via prefs

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P4)

Description

[Stefan Vassall]

Build ID: 2000062320


If you change the startup page using the preferences the tooltip for the home
button still says http://www.mozilla.org/. Clicking on the button takes you to
the new homepage.

Comment 1

[Stephen Walker]

confirming using 2000062320 nightly on w2k. if you change the url for the home 
button & don't restart, the tooltip still displays the prior home button url. 
but if you change the home button url & then restart mozilla the correct 
tooltip shows up

Comment 2

[Doron Rosenberg (IBM)]

throwing to UI:Feedback, ben should know where to send this to.

Comment 3

[Blake Ross]

the tooltip only seems to be updated when starting a new instance of the 
browser, or when setting a new homepage via dragging onto the home button.  
looks like we just need to update the tooltip upon pressing OK in the prefs, 
also.  right, ben? reassigning to me

Comment 4

[Blake Ross]

Ben, I was just thinking...if someone changes their homepage via prefs.js while 
the browsers running, afaik, the tooltip won't update.  This is a pretty minor 
deal, and the only way I can think of to fix it is to recheck what the tooltip 
should be every time the mouse passes over the home button...should we just not 
worry about this somewhat obscure case?

Comment 5

[Matthew T (active 1999-2002)]

Aren't prefs held in memory while Mozilla is running? So if someone changes their 
prefs.js manually (e.g. with a text editor) during run-time, it doesn't have any 
effect -- the prefs in memory aren't changed, and the prefs.js is overwritten 
whenever either (a) prefs are changed in the prefs dialog, or (b) Mozilla is 
quit.

Having said that, there's probably an API somewhere for setting prefs -- the 
tooltip should update whenever that API is called, rather than just in the case 
of the `Ok' button in the prefs dialog.

Comment 6

[Blake Ross]

That's right, I'm an idiot -- the tooltip won't change, but neither will the 
homepage.

Comment 7

[Blake Ross]

Actually - I think it only reads the pref when you click the button, so that 
might not be true. I'll look.

Comment 8

[Blake Ross]

I don't think this belongs in xpapps, but updating qa...

Comment 9

[sairuh (rarely reading bugmail)]

claudius, d'you know who owns this? chris or radha...?

Comment 10

[Claudius Gayle]

without knowing where the fix would come from (ie where in the code) I have know idea who would/could/should own this. 
Regardless, all of the likely candidates receive email on this bug, so I'm sure someone will step up :-).

Comment 11

[Blake Ross]

erm...why are we still trying to figure out who owns this when it's assigned to 
me? I'm not good enough ;) ?

Comment 12

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

need scriptable pref change callbacks before this can be fixed. 

Comment 13

[Blake Ross]

Do we have a bug on that?

Comment 14

[Blake Ross]

the fix for this is in bug 44239

Comment 15

[Blake Ross]

Fix checked in.

Comment 16

[Blake Ross]

turns out my fix didn't fix this, as I thought it did/would.  But I recall 
shaver saying pref callbacks in JS work now.  so I'll talk to him about it...

Comment 17

[Blake Ross]

other things to worry about right now.

Comment 18

[Markus Långström]

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

Comment 19

[Chris Lyon]

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

Comment 20

[Jim Crumley]

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

Comment 21

[Samir Gehani]

Nav triage team: nsbeta1+/adt3

Comment 22

[Kyle Yuan]

Attachment: register a pref callback function in nsXULDocument to handle the home page's change

I'm not sure whether the nsXULDocument is the best place to handle this change,
but at least, it can change the tooltiptext in *every* browser window without
an enumerating.

Comment 23

[Kyle Yuan]

Attachment: register a pref callback function in nsXULDocument to handle the home page's change

Sorry, my previous patch is wrong

Comment 24

[Christian :Biesinger (don't email me, ping me on IRC)]

this is, imho, a completely wrong approach for this bug... XULDocument should
not be used for such things...

why not register the callback in the Startup function (or equivalent) of
navigator.js?

Comment 25

[Kyle Yuan]

nsIPref::RegisterCallback is [noscript], so it can not be used in js code.

Comment 26

[Christian :Biesinger (don't email me, ping me on IRC)]

err, you should not use nsIPref anyway. use nsIPrefService / nsIPrefBranch.
the new way for adding a pref observer is scriptable:
http://lxr.mozilla.org/seamonkey/source/modules/libpref/public/nsIPrefBranchInternal.idl

Comment 27

[Kyle Yuan]

Attachment: pref callbak in navigator.js

okay, then the thing becomes quite easy :)

cbiesinger, could you r= my fix?

Comment 28

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 95673 [details] [diff] [review]
pref callbak in navigator.js

looks good and works fine,
r=biesi

Comment 29

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 95673 [details] [diff] [review]
pref callbak in navigator.js

So... it looks like setTooltipText (in globalOverlay.js) is no longer used
after this patch.  Could you check that there are no more users of it anywhere
and remove it if so?

Or you could keep using it...

Comment 30

[Kyle Yuan]

Attachment: use setTooltipText to set the tooltip

chekced in lxr, no other code uses setTooltipText. But I prefer to keep it here
for future use.

Comment 31

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 97287 [details] [diff] [review]
use setTooltipText to set the tooltip

sr=bzbarsky

Comment 32

[Kyle Yuan]

Comment on attachment 97287 [details] [diff] [review]
use setTooltipText to set the tooltip

Carrying out biesi's r=

Comment 33

[Kyle Yuan]

taking...

Comment 34

[Kyle Yuan]

checked in!

Comment 35

[Mike Kowalski]

verified using 2002083008 on Win XP.

Comment 36

[Chris Lyon]

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