584454 - Support MeegoTouch status bar in Fennec browser

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Steffen Imhof]

Attachment: Patch to support statusbar

The MeegoTouch framework allows a system statusbar to be shown at the top of the window.

The attached patch adds a component to Fennec which can be used to toggle this statusbar, furthermore a preference setting is provided, so that the user can switch it on and off in the GUI.

Because the MeegoTouch statusbar is shown on top of the browser window's content, there is a placeholder in the XUL code to make room for the statusbar.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

So the statusbar holds things like battery and wifi strength? and maybe some other system icons?

If so, I think this should be _always_ shown, much like it is on Android, unless we are in "fullscreen" mode.

I also think this should be built into the nsWindow code, not the XUL frontend. The nsWindow code should be aware of the system statusbar and the "fullscreen" mode and it should resize the screen accordingly.

Doug - Thoughts?

Comment 2

[Jeremias Bosch (:jbos)]

the statusbar is controlled and setuped by the system. The only thing we do is, enable and disable it. Thing is: we just moved it into fennec from nsWindow since it caused there crashes with remote tabs.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

I just don't think XUL tricks in the front-end are the best way to handle this. Resizing the window seems to be the best approach. If we handle this in the front-end, we'll need to put the XUL trick in every window we ever create.

Comment 4

[Jeremias Bosch (:jbos)]

Resize the window will move the statusbar since its a part of the window and not painted by the system. The Idea for a Version 2 is to use Layers and do it with a layout thingy.

Can we take that for now and create a follow up bug for a layer solution?

Comment 5

[Oleg Romashin (:romaxa)]

hmm... what do you think about this approach:

Create new XUL::embed or use existing NPAPI embed element, add that element as overlay to fennec UI.
And then paint MStatusBar into that embed element...
?

Comment 6

[Steffen Imhof]

Doesn't sound too bad to me. Is there some kind of element that could be used a "template" about what is to do, to create a new element?
Because at least I have not done much in the content/layout area yet.

(I have an alternate approach here I could upload soon, which is putting the statusbar handling back in xulrunner, but outside nsWindow. It seems to work, but is still kind of messy.)

Comment 7

[Steffen Imhof]

Attachment: WIP that uses a pure xulrunner approach, Fennec code is only needed for the statusbar toggle switch

This is a patch, which basically works by resizing the top level QGraphicsWidget according to the state of the preference. So on the Fennec side you only need the toggle in the setting GUI if you want to have it there.

It's an alternative approach to the pure Fennec implementation. It does not yet work by adding a new element.

The part I don't like yet is the fact that the height of the statusbar is needed in a few places for sizing computations.

Comment 8

[Steffen Imhof]

Attachment: WIP patch using resizing of the toplevel QGraphicsWidget

Sorry, last version was outdated. I attached the current one.

Comment 9

[Oleg Romashin (:romaxa)]

> Created attachment 465187 [details] [diff] [review]
> WIP patch using resizing of the toplevel QGraphicsWidget

Resizing toplevel widget looks too hacky and Qt only.

Can we just add extension with overlay, which will just allocate space in fennec-UI top|left|right|bottom and insert any element and have fennec correctly resized with that additional elements.

and then into allocated space we can add some hand-made statusbar, or MStatusBar or whatever we want.

Comment 10

[Miika Jarvinen]

Attachment: Move the statusbar handling to meego specific view

This patch takes bit different approach. Instead of using screenmanager, I use MozMSceneWindow to handle statusbar. This one is already meego specific class, so seems cleaner for me¸ to put it there. Instead of using preferences I listen for statusbar visiblity change events.

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 465880 [details] [diff] [review]
Move the statusbar handling to meego specific view

Style seems incorrect in many places, please fix that...

Comment 12

[Oleg Romashin (:romaxa)]

Comment on attachment 465880 [details] [diff] [review]
Move the statusbar handling to meego specific view

Functionality looks fine for me, it would be nice also check how it looks on desktop Firefox with meegotouch library enabled... but I think this is later.

Comment 13

[Miika Jarvinen]

Attachment: Move the statusbar handling to meego specific view

Fixed indentation as requested.

Comment 14

[Oleg Romashin (:romaxa)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

I think this is looks much better, and not so hacky comparing to previous version (with pref observers and screenManager hacks...)

With this patch we are enabling status bar and make it visible by default on meego platform... and we can hide/show it using simple extension (native or js-ctypes based).

mfinkle: do you have anything against this solution?

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

I like this approach. Let's get it reviewed. Adding DougT.

Oleg mentioned that there might be performance issues with showing the statusbar though.

Comment 16

[Doug Turner (:dougt)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

this may cause perf regressions while using raster/image drawing.  For example,
if the status bar has frequent changes there will be more painting.

>-    }
>+#ifndef MOZ_ENABLE_MEEGOTOUCH
>+    if (mIsTopLevel && XRE_GetProcessType() == GeckoProcessType_Default)
>+        GetViewWidget()->resize(aWidth, aHeight);
>+#endif
>+
>     mWidget->resize( aWidth, aHeight);


do you really want to resize twice here?


> 
>+#ifndef MOZ_ENABLE_MEEGOTOUCH
>     if (mIsTopLevel) {
>-#ifdef MOZ_ENABLE_MEEGOTOUCH
>-      if (XRE_GetProcessType() != GeckoProcessType_Default)
>+      if (XRE_GetProcessType() == GeckoProcessType_Default)
>+          GetViewWidget()->setGeometry(aX, aY, aWidth, aHeight);
>+    }
> #endif
>-      GetViewWidget()->setGeometry(aX, aY, aWidth, aHeight);
>-    }
>+

same question

Comment 17

[Jeremias Bosch (:jbos)]

(In reply to comment #16)

> this may cause perf regressions while using raster/image drawing.  For example,
> if the status bar has frequent changes there will be more painting.

There are many ways to handle this. Right now i'm not so curious to fix it, since we still wait on Layers, Graphicsystem changes, and other things to take place. When we have a better picture about the different solutions we can pick the best and be done.

This is definitely a serious issue, but for now its not a blocking one. Its more important to have the statusbar in since this is a bug on our side if not. If the statusbar is in and its somehow slow we can forward to the framework and have it fixed with the best possible solution. But it does not help to keep it from getting in now.

Comment 18

[Oleg Romashin (:romaxa)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

I think it is ok now

Comment 19

[Doug Turner (:dougt)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

until my questions are answered

Comment 20

[Oleg Romashin (:romaxa)]

> this may cause perf regressions while using raster/image drawing.  For example,

I hope nokia and intel will take care about it.



> >-    }
> >+#ifndef MOZ_ENABLE_MEEGOTOUCH
> >+    if (mIsTopLevel && XRE_GetProcessType() == GeckoProcessType_Default)
> >+        GetViewWidget()->resize(aWidth, aHeight);
> >+#endif
> >+
> >     mWidget->resize( aWidth, aHeight);
> 
> 
> do you really want to resize twice here?

It is not twice, it is 2 different things, GraphicsView and GraphicsWidget 

> >+          GetViewWidget()->setGeometry(aX, aY, aWidth, aHeight);
> >+    }
> > #endif
> 
> same question

I think the same answer ;)

Comment 21

[Oleg Romashin (:romaxa)]

Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

Non part of default build and needed for Fennec on Meego

Comment 22

[Oleg Romashin (:romaxa)]

http://hg.mozilla.org/mozilla-central/rev/09d3d9562e61