Closed Bug 584454 Opened 14 years ago Closed 14 years ago

Support MeegoTouch status bar in Fennec browser

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
MeeGo
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: steffen.imhof, Unassigned)

References

Details

Attachments

(2 files, 3 obsolete files)

Attached patch Patch to support statusbar (obsolete) — Splinter Review
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.
Attachment #462868 - Flags: review?(mark.finkle)
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?
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.
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.
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?
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...
?
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.)
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.
Depends on: 585687
Sorry, last version was outdated. I attached the current one.
Attachment #464072 - Attachment is obsolete: true
> 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.
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.
Attachment #465880 - Flags: review?(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 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.
Fixed indentation as requested.
Attachment #465880 - Attachment is obsolete: true
Attachment #466428 - Flags: review?(romaxa)
Attachment #465880 - Flags: review?(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?
Attachment #466428 - Flags: feedback?(mark.finkle)
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.
Attachment #466428 - Flags: review?(doug.turner)
Attachment #466428 - Flags: feedback?(mark.finkle)
Attachment #466428 - Flags: feedback+
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
(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 on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

I think it is ok now
Attachment #466428 - Flags: review?(romaxa) → review+
Comment on attachment 466428 [details] [diff] [review]
Move the statusbar handling to meego specific view

until my questions are answered
Attachment #466428 - Flags: review?(doug.turner) → review-
> 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 ;)
Attachment #466428 - Flags: review- → review+
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
Attachment #466428 - Flags: approval2.0?
http://hg.mozilla.org/mozilla-central/rev/09d3d9562e61
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attachment #466428 - Flags: approval2.0?
Attachment #462868 - Attachment is obsolete: true
Attachment #462868 - Flags: review?(mark.finkle)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: