issues when starts with collapsed sidebar

RESOLVED FIXED in seamonkey2.16

Status

SeaMonkey
Sidebar
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: Dmitry Butskoy, Assigned: Karsten Düsterloh)

Tracking

Trunk
seamonkey2.16
x86_64
Linux

SeaMonkey Tracking Flags

(seamonkey2.16 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121016 Firefox/16.0 SeaMonkey/2.13.1
Build ID: 20121016181020

Steps to reproduce:

I start the browser with sidebar enabled, but "collapsed"
(ie. the sidebar splitter is just at the left border, or sidebar width "is zero", etc.)

To enable such a setup, start browser, unhide the sidebar (F9), then collaps it (to "zerolength") by mouse. Then exit browser.

Now start the browser (with sudebar collapsed), select some text on a page (by left click), and open context menu on selected text by right click.


Actual results:

When you start browser with collapsed sidebar, the function

suite/common/sidebar/sudebarOverlay.js:sidebar_overlay_init()

does nothing about initialization.
It leads to (at least) broken context menu (right click) on a selected text on a page. Instead of the proper entries ("Copy", "Select All", "Search Google", "View Selection Source") you see entries from whole seamonkey components...  The Error Console shows some more info on this exact case.

If you uncollaps the sidebar (once at the session), or even collaps them again (at the same sassion), the symptoms gone. The reason is once uncollapsed, the sidebar things are initialized properly.
(Reporter)

Comment 1

5 years ago
Created attachment 671868 [details] [diff] [review]
The proposed patch.

This patch fixes the issue for me (I maintain Seamonkey for Fedora EPEL6 project).

Seems EasyFix.

Note that sidebar_overlay_init() has one more sidebar_is_collapsed() call, was never reached due to the removed one by the patch.

Also note, that hidden/unhidden state does not influence the issue, even hidden, if the state is collapsed, the problem occurs. To fix the issue manually, you should first unhide the sidebar, the uncollaps it, then hide it again in uncollapsed state.

Updated

5 years ago
Attachment #671868 - Flags: review?(mnyromyr)

Comment 2

5 years ago
Hello Dmitry. Thank you for your patch. If you haven't already read these documents I recommend that you do so:
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F
https://developer.mozilla.org/en-US/docs/Creating_a_patch
(Reporter)

Comment 3

5 years ago
Created attachment 673202 [details] [diff] [review]
The same patch against trunk, generated by hg diff as suggested in comment #2

The same patch against the current comm-central tree.

Comment 4

5 years ago
Comment on attachment 673202 [details] [diff] [review]
The same patch against trunk, generated by hg diff as suggested in comment #2

Thanks for the updated patch. Setting review requested flag.
Attachment #673202 - Flags: review?(mnyromyr)

Updated

5 years ago
Attachment #671868 - Attachment is obsolete: true
Attachment #671868 - Flags: review?(mnyromyr)
(Assignee)

Updated

5 years ago
Assignee: nobody → dmitry
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Assignee)

Comment 5

5 years ago
Comment on attachment 673202 [details] [diff] [review]
The same patch against trunk, generated by hg diff as suggested in comment #2

Dmitry, thanks for touching this, sidebar is definitely in need of some loving'!

Alas, your patch is not the right fix — the code you are removing was added in bug 72208 to allow lazy initialization of the sidebar panels, especially when sidebar is present but collapsed. 
The actual problem can be seen reported on the Error Console:

Timestamp: 11/04/2012 07:51:16 PM
Warning: ReferenceError: reference to undefined property sidebarObj.panels
Source File: chrome://navigator/content/navigator.js
Line: 1181
Timestamp: 11/04/2012 07:51:16 PM
Error: TypeError: sidebarObj.panels is undefined
Source File: chrome://navigator/content/navigator.js
Line: 1181

Obviously, the searchSidebar getter needs to take precautions against a non-initialized panels member.
Attachment #673202 - Flags: review?(mnyromyr) → review-
(Reporter)

Comment 6

5 years ago
Well, the bug 72208 ia 12 years old now.

Whether the issue described there still actual today? Was the bug 72208 about an actual bug, or about an enhancement?

IOW, if it is related to an enhancement (an optimizing feature, not a bug fixing), then it seems that due to the enhancement actual for 2001, we still provide the buggy code in 2012...

I am whining here because I am not so skill in Mozilla to fix the problem some another way... :)
(Assignee)

Comment 7

5 years ago
Created attachment 678825 [details] [diff] [review]
stabilize searchSidebar getter

(In reply to Dmitry Butskoy from comment #6)
> Whether the issue described there still actual today? Was the bug 72208
> about an actual bug, or about an enhancement?

Well, both. If you have slow loading or work intensive sidebars, it doesn't make much sense to load something you can't see. And it might even slow down SeaMonkey for no obvious reason.

> I am whining here because I am not so skill in Mozilla to fix the problem
> some another way... :)

I meant what I wrote literally, see attached patch. ;-)
Assignee: dmitry → mnyromyr
Attachment #673202 - Attachment is obsolete: true
Attachment #678825 - Flags: review?(iann_bugzilla)
(Reporter)

Comment 8

5 years ago
Well, new patch fix the issue for me.


But I still see in error console some message (both with the new and prevoius patch):

Error: Search service falling back to synchronous initialization at SRCH_SVC__ensureInitialized@resource:///components/nsSearchService.js:2498
@resource:///components/nsSearchService.js:3468
@chrome://communicator/content/nsContextMenu.js:1284
@chrome://communicator/content/nsContextMenu.js:45
nsContextMenu@chrome://communicator/content/nsContextMenu.js:26
onpopupshowing@chrome://navigator/content/navigator.xul:1

Source File: resource:///components/nsSearchService.js
Line: 2499

Don't sure whether it is related to my specific user config or not...

Comment 9

5 years ago
> Error: Search service falling back to synchronous initialization at

This is a harmless error message and is fixed in SeaMonkey 2.14.
See Bug 780179 - SeaMonkey should use the asynchronous API from nsIBrowserSearchService.

Updated

5 years ago
Attachment #678825 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 10

5 years ago
http://hg.mozilla.org/comm-central/rev/5751ab0e7c98
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Version: SeaMonkey 2.13 Branch → Trunk
(Reporter)

Comment 11

4 years ago
I do not see the patch applied in the latest seamonkey-2.15,
whereas the report was at 2.13 time...

In what version of seamonkey the fix of this issue should appear?
(In reply to Dmitry Butskoy from comment #11)
> I do not see the patch applied in the latest seamonkey-2.15,
> whereas the report was at 2.13 time...
> 
> In what version of seamonkey the fix of this issue should appear?

The fix was landed between the 2012-11-12 and 2012-11-13 nightlies of SeaMonkey 2.16a1 (see comment #10 and http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2012/11/2012-11-13-00-30-05-comm-central-trunk/ which should be the first set of nightlies with the fix). The current 2.16 beta, and, in six weeks or so, the upcoming 2.16 release, should have the fix.
status-seamonkey2.16: --- → fixed
Target Milestone: --- → seamonkey2.16
You need to log in before you can comment on or make changes to this bug.