Last Comment Bug 802166 - issues when starts with collapsed sidebar
: issues when starts with collapsed sidebar
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Sidebar (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: seamonkey2.16
Assigned To: Karsten Düsterloh
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-16 08:08 PDT by Dmitry Butskoy
Modified: 2013-01-14 04:59 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed


Attachments
The proposed patch. (1.09 KB, patch)
2012-10-16 08:17 PDT, Dmitry Butskoy
no flags Details | Diff | Splinter Review
The same patch against trunk, generated by hg diff as suggested in comment #2 (2.06 KB, patch)
2012-10-19 06:02 PDT, Dmitry Butskoy
mnyromyr: review-
Details | Diff | Splinter Review
stabilize searchSidebar getter (941 bytes, patch)
2012-11-06 11:30 PST, Karsten Düsterloh
iann_bugzilla: review+
Details | Diff | Splinter Review

Description Dmitry Butskoy 2012-10-16 08:08:57 PDT
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.
Comment 1 Dmitry Butskoy 2012-10-16 08:17:29 PDT
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.
Comment 2 Philip Chee 2012-10-18 10:56:32 PDT
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
Comment 3 Dmitry Butskoy 2012-10-19 06:02:20 PDT
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 Philip Chee 2012-10-19 12:38:37 PDT
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.
Comment 5 Karsten Düsterloh 2012-11-04 11:08:08 PST
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.
Comment 6 Dmitry Butskoy 2012-11-06 04:14:35 PST
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... :)
Comment 7 Karsten Düsterloh 2012-11-06 11:30:05 PST
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. ;-)
Comment 8 Dmitry Butskoy 2012-11-07 05:59:39 PST
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 Philip Chee 2012-11-08 08:38:25 PST
> 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.
Comment 10 Karsten Düsterloh 2012-11-12 14:40:18 PST
http://hg.mozilla.org/comm-central/rev/5751ab0e7c98
Comment 11 Dmitry Butskoy 2013-01-09 09:41:10 PST
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?
Comment 12 Tony Mechelynck [:tonymec] 2013-01-14 04:59:28 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.