Last Comment Bug 467867 - (SMtabAPI) [META] Implement Firefox TabBrowser API in Seamonkey
(SMtabAPI)
: [META] Implement Firefox TabBrowser API in Seamonkey
Status: NEW
: meta
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: All All
: -- enhancement with 3 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
https://developer.mozilla.org/en/XUL/...
Depends on: 482291 484968 554908 558614 558673 558995 570783 570791 570981 573733 577756 579845 586340 595483 609034 633121
Blocks: smtabmail
  Show dependency treegraph
 
Reported: 2008-12-03 19:41 PST by D
Modified: 2011-05-07 12:30 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description D 2008-12-03 19:41:53 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081203 Lightning/0.6a1 SeaMonkey/2.0a3pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081203 Lightning/0.6a1 SeaMonkey/2.0a3pre

Firefox tabbrowser API(s) needs to be implement SeaMonkey.
More extension will be compatible with Seamonkey because of tab menu and behavior of tabs.
We could use various extensions for behavior of tabs ...



Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Actual Results:  




...for now we have that we can't use no-one extension for rule tabs...
Comment 1 Tyler Downer [:Tyler] 2008-12-04 11:07:14 PST
not sure if this is practical now, but wait for a dev...
Comment 2 Karsten Düsterloh 2008-12-23 16:41:23 PST
Basically, I kind of need this to base tabmail upon: tabmail uses a bunch of prefs stolen and renamed from the FF tabbrowser implementation. 

We _could_ implement whacky mappings from our tabbrowser prefs to the ones used/needed by tabmail, but I think tabbrowser has some modifications worthwile to take over, eg. close button prefs and open tab list.

Marking as blocking the tabmail, but that's not a hard block - tabmail works perfectly without it, just the pref defaults are missing, causing odd default sizing.
Comment 3 Philip Chee 2008-12-23 20:41:53 PST
> Marking as blocking the tabmail, but that's not a hard block - tabmail works
> perfectly without it, just the pref defaults are missing, causing odd default
> sizing.

I'd suggest filing a more specific bug about the firefox tabbrowser prefs. This should be a meta/tracking bug instead.
Comment 4 Tony Mechelynck [:tonymec] 2008-12-24 06:43:47 PST
Not sure I like the direction the Fx tabbrowser is evolving. OTOH it would of course facilitate maintenance to have only one tabbrowser subsystem rather than two, and maybe it would even allow using the Tab Mix Plus extension (one of my Fx must-haves) with SeaMonkey.
Comment 5 Philip Chee 2008-12-24 09:18:31 PST
We aren't talking about the UI, just the APIs so that Firefox extensions could be more easily ported over.
Comment 6 D 2009-01-05 06:40:32 PST
It's almost what you want, Tony. TabMixPlus should work with FF tabbrowser API calls(may be with some modification).
And anyone knows should need to port FF UI or part of UI to SM ?
like examle:
Does SM UI support vertical&horizontal scrollbars in tab aria ? (like supports in TabKit extension).
Comment 7 Philip Chee 2009-03-06 07:42:31 PST
Lets turn this into a meta bug and file separate dependent bugs for each specific issue.
Comment 8 Robert Kaiser 2010-04-06 06:06:13 PDT
Misak, you said you might want to look into this?
Comment 9 Misak Khachatryan 2010-04-06 06:15:29 PDT
Yes, but it's not a high priority for me. First i want to implement mailnews sessionstore, keep parity with firefox sessionstore (there is a bunch of patches waiting to port), and then tabbrowser API. If it should be done at specific time, i can't guarantee that, otherwise I'll work on it.
Comment 10 Robert Kaiser 2010-04-13 06:25:49 PDT
https://developer.mozilla.org/en/XUL/tabbrowser has a documentation of much of the public API, I guess we should try to implement the items listed there, as that's as close to a public API doc as it gets.
Comment 11 Misak Khachatryan 2010-04-19 03:52:25 PDT
I checked both tabbrowser implementations, we miss 3 methods - loadOneTab, loadTabs and selectTabAtIndex. addTab have parameter differences, all others in sync. Bug 558614 for loadTabs, where i fixed all mentioned stuff above, except selectTabAtIndex. The last should be very easy. I'll do it in separate bug.
Comment 12 Robert Kaiser 2010-04-19 06:08:07 PDT
(In reply to comment #11)
> I checked both tabbrowser implementations, we miss 3 methods - loadOneTab,
> loadTabs and selectTabAtIndex.

What about things like getIcon() or the tab progress listeners?
Comment 13 Misak Khachatryan 2010-04-19 06:14:56 PDT
Tab progress listeners are waiting for checkin, regarded getIcon() - I've checked only methods listed in https://developer.mozilla.org/en/XUL/tabbrowser. If someone gets a list of other useful methods, I'll port them too.
Comment 14 Robert Kaiser 2010-04-19 06:27:19 PDT
(In reply to comment #13)
> I've
> checked only methods listed in https://developer.mozilla.org/en/XUL/tabbrowser.

Ah, I parsed "I checked both tabbrowser implementations" wrongly, then. I thought you looked at both tabbrowser.xml files and compared them.
Comment 15 Misak Khachatryan 2010-04-19 06:31:31 PDT
I did cat mozilla/browser/base/content/tabbrowser.xml | grep "method name" -A 10 for both files and looked to parameters, but didn't carefully examined methods not listed in the wiki. But they look very similar. So - just give me the list ;)
Comment 16 Michael Kraft [:morac] 2010-06-07 07:45:39 PDT
Is gBrowser.addTabsProgressListener fully implemented currently in the Trunk?  The reason I ask is that while I am getting most of the listener callbacks mentioned on https://developer.mozilla.org/En/Listening_to_events_on_all_tabs I am not getting the onLinkIconAvailable one.  I don't know if that means it's simply not being called or link icons are never loading.  I think it might be the later since I'm noticing that icons aren't displaying in tabs for most web sites.
Comment 17 neil@parkwaycc.co.uk 2010-06-07 07:51:11 PDT
Certainly if you don't see the link icon for a site then you won't get the notification either ;-) Could the problem be a conflict with another addon? Do you see the link icons when running in safe mode?
Comment 18 Michael Kraft [:morac] 2010-06-07 09:26:23 PDT
Safe mode doesn't seem to be working (-safe-mode parameter doesn't do anything) so I created a new profile and icons aren't loading for a number of sites in that one either.  For example http://www.google.com doesn't load an icon, but http://www.seamonkey-project.org/start/ does.  This isn't limited to the trunk load, I see the same thing in SeaMonkey 2.0.4 and 2.0.6pre.  I'm guessing that's a different issue then.

I decided to do some digging into the tabbrowser.xml code (http://mxr.mozilla.org/comm-central/source/suite/browser/tabbrowser.xml#355) and found that it is not calling onRefreshAttempted or onLinkIconAvailable for this.mTabBrowser.mTabsProgressListeners stored in tabbrowser.xml.  All the other TabProgressListener callbacks are called.

onRefreshAttempted needs to be added around line 642 (in onRefreshAttempted function) and onLinkIconAvailable needs to be added around line 648 (in setIcon function).
Comment 19 neil@parkwaycc.co.uk 2010-06-07 09:38:33 PDT
(In reply to comment #18)
> Safe mode doesn't seem to be working (-safe-mode parameter doesn't do anything)
> so I created a new profile and icons aren't loading for a number of sites in
> that one either.  For example http://www.google.com doesn't load an icon, but
> http://www.seamonkey-project.org/start/ does.  This isn't limited to the trunk
> load, I see the same thing in SeaMonkey 2.0.4 and 2.0.6pre.  I'm guessing
> that's a different issue then.
We only call onLinkIconAvailable for link icons. Google uses a favicon.

> I decided to do some digging into the tabbrowser.xml code
> (http://mxr.mozilla.org/comm-central/source/suite/browser/tabbrowser.xml#355)
> and found that it is not calling onRefreshAttempted or onLinkIconAvailable for
> this.mTabBrowser.mTabsProgressListeners stored in tabbrowser.xml.  All the
> other TabProgressListener callbacks are called.
Actually onRefreshAttempted seems to be called twice on the progress listeners instead of once on the progress listeners and once on the tabs progress listeners. That's a bug, and thanks for finding it. Do you want to file it?

> onRefreshAttempted needs to be added around line 642 (in onRefreshAttempted
> function) and onLinkIconAvailable needs to be added around line 648 (in setIcon
> function).
Sure, but I think the existing notification should also move there too.
Comment 20 Michael Kraft [:morac] 2010-06-07 16:46:46 PDT
I guess I could file it.  Should I include the part about the missing onLinkIconAvailable for tab progress listeners as well or should that be separate?
Comment 21 neil@parkwaycc.co.uk 2010-06-08 11:22:32 PDT
(In reply to comment #20)
> I guess I could file it.  Should I include the part about the missing
> onLinkIconAvailable for tab progress listeners as well or should that be
> separate?
It should be separate, since the refresh bug is trivial by comparison.
Comment 22 Michael Kraft [:morac] 2010-06-08 11:59:12 PDT
Okay the onRefreshAttempt bug is bug 570783.

The onLinkIconAvailable bug is bug 570791.  This will probably require
gBrowser.setIcon() to be implemented, which would be a separate bug.
Comment 23 Robert Kaiser 2010-06-08 12:22:31 PDT
.setIcon() is already in bug 554908.
Comment 24 Michael Kraft [:morac] 2010-06-08 12:58:55 PDT
(In reply to comment #23)
> .setIcon() is already in bug 554908.

So it is.  Looks like the onLinkIconAvailable code is commented out though so I guess bug 570791 is still valid.  Actually it looks like you could kill two bugs with one stone, if you moved the code from setIcon and updateIcon into mTabProgressListener.setIcon().  I'll comment about the specifics in bug 554908.
Comment 25 Robert Kaiser 2010-06-08 13:23:09 PDT
(In reply to comment #24)
> So it is.  Looks like the onLinkIconAvailable code is commented out though

That's just because .mTabsProgressListeners was not implemented on the suite side yet when I created the patch. Now, it can just be left in as it should be.
Comment 26 Michael Kraft [:morac] 2010-06-08 14:20:09 PDT
(In reply to comment #25)
> That's just because .mTabsProgressListeners was not implemented on the suite
> side yet when I created the patch. Now, it can just be left in as it should be.

It still won't work as currently written though because .mTabsProgressListeners isn't implemented exactly the same way it is in Firefox.  See my reply in bug 554908.
Comment 27 Robert Kaiser 2010-06-08 18:25:46 PDT
(In reply to comment #26)
> It still won't work as currently written though because .mTabsProgressListeners
> isn't implemented exactly the same way it is in Firefox.

Then that cause is the bug. We ought to be implementing the same API as Firefox or this very bug here is useless.
Comment 28 Michael Kraft [:morac] 2010-06-08 18:27:49 PDT
(In reply to comment #27)
> Then that cause is the bug. We ought to be implementing the same API as Firefox
> or this very bug here is useless.

From looking at the code, it looks like the API will work the same, it's just the implementation that's different.
Comment 29 Philip Chee 2010-06-09 00:49:40 PDT
Bug 562649 set and correctly handle userTypedValue when loading external URIs
http://hg.mozilla.org/mozilla-central/rev/f2070673fdbc
Comment 30 Jens Hatlak (:InvisibleSmiley) 2010-06-17 11:39:46 PDT
(In reply to comment #11)
> I checked both tabbrowser implementations, we miss 3 methods - loadOneTab,
> loadTabs and selectTabAtIndex. addTab have parameter differences, all others in
> sync. Bug 558614 for loadTabs, where i fixed all mentioned stuff above, except
> selectTabAtIndex. The last should be very easy. I'll do it in separate bug.

Care to file it? :-)

[Big thanks for your work BTW!]
Comment 31 Justin Wood (:Callek) 2011-05-07 11:15:37 PDT
Sooo... do we intend to continue along with this meta bug, or do we want to resolve as fixed.

All deps are fixed by now!
Comment 32 Philip Chee 2011-05-07 12:30:36 PDT
Bug 570981 is a firefox bug.

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