Closed Bug 220681 Opened 18 years ago Closed 17 years ago
New window uses small icons even though original window has large icons
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
> 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
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
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
Assignee: bugs → sspitzer
Assignee: sspitzer → mconnor
Attachment #132418 - Flags: review?(mconnor) → review+
fixed, branch and trunk
Assignee: mconnor → dkoppenh
done, sorry about the incredibily long delay! :)
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.