Closed Bug 220681 Opened 18 years ago Closed 17 years ago

New window uses small icons even though original window has large icons

Categories

(Firefox :: Toolbars and Customization, defect, P2)

x86
Windows 98
defect

Tracking

()

RESOLVED FIXED
Firefox1.0beta

People

(Reporter: david, Assigned: david)

Details

(Keywords: fixed-aviary1.0)

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030904 Firebird/0.6.1+
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.5b) Gecko/20030904 Firebird/0.6.1+

New window uses small icons even though original window has large icons:

If iconsize="small" is in localstore, and then you switch to large icons, new
windows you launch will still have small icons.  This condition continues until
you close a window that has large icons, after which new windows will have large
icons.

I fixed this issue as a part of my patch for bug 173444.  However, that
particular part of the patch was not checked in, so this is the spinoff bug. 
The issue, I believe, was first mentioned in bug 171731 comment 1 and bug 171731
comment 2.


Reproducible: Always

Steps to Reproduce:
0. Create / Use new profile.
1. Observe localstore.rdf has no toolbar info in it.
2. View -> Toolbars -> Customize...
3. Uncheck "Use Small Icons" checkbox.
4. Press "Done" button.
5. Observe localstore.rdf has iconsize="" for browser.xul#toolbar-menubar,
#navigator-toolbox, #PersonalToolbar, and #nav-bar
6. View -> Toolbars -> Customize...
7. Check "Use Small Icons" checkbox.
8. Press "Done" button.
9. Observe localstore.rdf has iconsize="small" for the aforementioned elements.
10. View -> Toolbars -> Customize...
11. Uncheck "Use Small Icons" checkbox.
12. Press "Done" button.
13. Observe localstore.rdf has no toolbar iconsize info in it.

So, even though the toolbar icons are currently large (or rather, 'not small'),
there is no indication in localstore.rdf that this is their state.

14. File -> New Window

Actual Results:  
A new window appears with small icons.  This is because there is no information
currently in localstore.rdf, and the default for the toolbox and toolbars in
browser.xul is iconsize="small".


Expected Results:  
A new window appears with large icons (the same size that the first window has).


The problem happens because the setAttribute function in customizeToolbar.js
will _remove_ the attribute if null is passed as the value, and null is used to
represent a switch to "large" icons in updateIconSize(aUseSmallIcons):

  var val = aUseSmallIcons ? "small" : null;
  setAttribute(gToolbox, "iconsize", val);

The issue is worked around if we s/null/"large"/ above, so that the custom
setAttribute() does not remove the attribute.

Alternately (though I have not yet tested this), we could use the DOM's
setAttribute instead of the custom function:  gToolbox.setAttribute("iconsize",
val); etc...

I'll attach a patch later.

David
Status: UNCONFIRMED → NEW
Ever confirmed: true
> The problem happens because the setAttribute function in customizeToolbar.js
> will _remove_ the attribute if null is passed as the value, and null is used
> to represent a switch to "large" icons in updateIconSize(aUseSmallIcons):

OK, this is sort of a lie.

I changed from the custom setAttribute function to the DOM version instead, and
got the same buggy results.

This prompted me to look closer at exactly what was being stored in
localstore.rdf, and when.

Now, I think that the real 'problem' is that if you try to persist a blank
(null) value _when a value is already persisted_, the persistance is deleted. 
If you persist a null value when there is _not_ a value already persisted, the
null value _is_ persisted!

This seems to explain why the first 'switch' to large icons actually works WRT
new windows.  There is no persistance, then we switch to persisting "" (large),
and that works.  Then, we switch back to small icons (persist "small", which
also works).  Finally, attempting to persist the "" (large) value again (while
currently persisting "small") removes the persistance this time.

The workaround of s/null/"large"/ in the line:
  var val = aUseSmallIcons ? "small" : null;
will work, because we would not attempt to persist a null value anymore.

David
Attachment #132418 - Flags: review?(hyatt)
Dont know if this is the correct bug, I have use big icons, but i get small
icons when I
1) righ-click to customize
2) click on "use small icons" to uncheck it, then click it again
3) restart browser --> icons are small
Flags: blocking1.0?
Attachment #132418 - Flags: review?(hyatt) → review?(bugs)
Assignee: hyatt → bugs
don't reassign to someone else, if you're working on the bug, you should be the 
one who its assigned to.

also, this seems sane and simple enough, I'll get to this during the week.
+ing since there's a patch
Flags: blocking1.0? → blocking1.0+
The reason I reassigned to Ben was that I don't have the time or setup available
ATM to update / revise the patch.

Thanks for taking a look at it.

David
Comment on attachment 132418 [details] [diff] [review]
avoid persisting a null value for iconsize

moving review to myself so I remember to do this.
Attachment #132418 - Flags: review?(bugs) → review?(mconnor)
Priority: -- → P2
Target Milestone: --- → Firefox1.0beta
Flags: blocking-aviary1.0RC1+
Assignee: bugs → sspitzer
Assignee: sspitzer → mconnor
Assignee: mconnor → mconnor
Attachment #132418 - Flags: review?(mconnor) → review+
fixed, branch and trunk
Assignee: mconnor → dkoppenh
Keywords: fixed-aviary1.0
done, sorry about the incredibily long delay! :)
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
QA Contact: bugzilla → toolbars
You need to log in before you can comment on or make changes to this bug.