Closed
Bug 68541
Opened 24 years ago
Closed 21 years ago
no checkmark for View|Toolbar in bookmarks manager
Categories
(SeaMonkey :: Bookmarks & History, defect, P5)
Tracking
(Not tracked)
RESOLVED
FIXED
Future
People
(Reporter: bzbarsky, Assigned: bugs)
References
Details
(Keywords: helpwanted, polish, Whiteboard: [br])
Attachments
(2 files, 3 obsolete files)
(deleted),
image/jpeg
|
Details | |
(deleted),
patch
|
p_ch
:
review+
p_ch
:
superreview+
|
Details | Diff | Splinter Review |
BUILD: Linux 2001-02-11-06 and Windows 2001-02-10
STEPS TO REPRODUCE:
1) open bookmark manager
2) open the view menu.
EXPECTED RESULT:
See a checkmark next to the "Toolbar" option
ACTUAL RESULT:
No checkmark.
Switching the toolbar on and off does not fix this.
Comment 2•24 years ago
|
||
Toggling the toolbar (switching on->off, off->on) "fixes" this. But it's still a
problem. Bookmarks Manager should check at startup whether toolbar is showed and
then check/uncheck based on that information.
Downgrading to trivial
Updated•24 years ago
|
Whiteboard: [br]
Comment 3•24 years ago
|
||
Wieeerd. I tried to add a
dump(document.getElementById("command-toolbar").hidden);
in the Startup() function of bookmarks.js, but it returned
"undefined".
Also (document.getElementById("viewCommandToolbar").checked is undefined.
If the "hidden" attribute of the toolbar wasn't undefined when the window is
launched, the checkmark would probably be there. Any idea? bz? hwaara?
Comment 4•23 years ago
|
||
Comment 5•23 years ago
|
||
Makes sense. r=hwaara
Assignee | ||
Comment 6•23 years ago
|
||
Status: NEW → ASSIGNED
Comment 7•23 years ago
|
||
a=dbaron for trunk checkin
Reporter | ||
Comment 8•23 years ago
|
||
Fix checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 9•23 years ago
|
||
Seeing this on new installs of build 2001100303 on Win2K.
Checking and unchecking fixes the problem, so it looks like a one-time issue,
but it's back.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 10•23 years ago
|
||
Assignee | ||
Updated•23 years ago
|
Assignee | ||
Comment 11•23 years ago
|
||
Paul Chen is now taking Bookmarks bugs. For your convenience, you can filter
email notifications caused by this by searching for 'ilikegoats'.
Assignee: ben → pchen
Status: ASSIGNED → NEW
Comment 12•23 years ago
|
||
Mass move Ben's bugs dumped on me marked future with p5 to get off my untriaged
radar. You can filter out this email by looking for "ironstomachaussie"
Priority: -- → P5
Comment 14•23 years ago
|
||
removing self from cc list
Comment 15•22 years ago
|
||
Not seeing this in 20021229 win2k.
Comment 16•22 years ago
|
||
Attachment #36233 -
Attachment is obsolete: true
Comment 17•22 years ago
|
||
Migrating patch from bug 203818
Same as timeless' patch except the cleanup and the backing out of the previous
patch. the persistent attribute is already set in goToggleToolbar.
Attachment #111117 -
Attachment is obsolete: true
Updated•22 years ago
|
Attachment #122052 -
Flags: superreview?(jaggernaut)
Attachment #122052 -
Flags: review?(varga)
Comment 18•22 years ago
|
||
Comment on attachment 122052 [details] [diff] [review]
patch v1.0
looks good
Attachment #122052 -
Flags: review?(varga) → review+
Comment 19•22 years ago
|
||
Comment on attachment 122052 [details] [diff] [review]
patch v1.0
>Index: communicator/resources/content/utilityOverlay.js
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/communicator/resources/content/utilityOverlay.js,v
>retrieving revision 1.74
>diff -u -r1.74 utilityOverlay.js
>--- communicator/resources/content/utilityOverlay.js 14 Mar 2003 09:23:14 -0000 1.74
>+++ communicator/resources/content/utilityOverlay.js 29 Apr 2003 19:28:06 -0000
>@@ -160,29 +160,19 @@
>
> function goToggleToolbar( id, elementID )
> {
>+ var toolbar = document.getElementById(id);
>+ var element = document.getElementById(elementID);
>+ if (toolbar)
> {
>+ var isShown = !toolbar.hidden;
>+ toolbar.hidden = isShown;
> document.persist(id, 'hidden');
>+ if (element) {
>+ element.checked = isShown;
>+ document.persist(elementID, 'checked');
>+ }
> }
> }
This doesn't seem right.
If currently the toolbar is hidden (isShown = false), we unhide it (hidden =
false), and we set checked to false (checked = false).
Shouldn't that be |element.checked = !isShown|?
Comment 20•22 years ago
|
||
Comment on attachment 122052 [details] [diff] [review]
patch v1.0
sr=jag with that bug fixed.
Attachment #122052 -
Flags: superreview?(jaggernaut) → superreview+
Comment 22•22 years ago
|
||
Comment on attachment 122061 [details] [diff] [review]
patch v1.1
Carrying reviews
Requesting approval:
Risk is low.
Strictly speaking, this patch has not been tested on Seamonkey, but it has been
tested on phoenix and consumers of |goToggleToolbar| have been checked. (codes
are the same in the patch scope).
Attachment #122061 -
Flags: superreview+
Attachment #122061 -
Flags: review+
Attachment #122061 -
Flags: approval1.4b?
Comment 23•22 years ago
|
||
Comment on attachment 122061 [details] [diff] [review]
patch v1.1
have you smoke tested the toolbars in browser, mail, addrbook, compose, etc?
Comment 24•22 years ago
|
||
Comment on attachment 122061 [details] [diff] [review]
patch v1.1
"this patch has not been tested on Seamonkey"
seek approval after testing.
we can't a= something that hasn't even been tested.
Attachment #122061 -
Flags: approval1.4b?
Comment 25•21 years ago
|
||
Comment on attachment 122061 [details] [diff] [review]
patch v1.1
>+ element.checked = isHidden;
Maybe in Firebird menuitems have a checked property but in Mozilla they do not,
patch is ok if you use element.setAttribute("checked", isHidden);
Comment 26•21 years ago
|
||
I think this was fixed by Jan's checkin to bug 205378.
Status: NEW → RESOLVED
Closed: 23 years ago → 21 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•