770845 - Enable Maemo5 support with Qt/XUL

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[Steven Newbury]

Attachment: fennec_qt_maemo5.patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20100101 Firefox/14.0
Build ID: 20120702083010

Steps to reproduce:

Fennec was originally ported to Maemo5 using GTK/XUL.  The GTK port development has stalled, and the current ports for Maemo6 and MeeGo use Qt/XUL.


Actual results:

Fennec Qt/XUL fails to build on Maemo5, and lacks support for native Hildon rotation.


Expected results:

To take advantage of improvements and with a modern Qt being available on Maemo5, we've ported the Qt/XUL version, including support for native rotation.

Comment 1

[Oleg Romashin (:romaxa)]

Comment on attachment 639052 [details] [diff] [review]
fennec_qt_maemo5.patch

>diff --git a/widget/qt/mozqwidget.h b/widget/qt/mozqwidget.h
>old mode 100644
>new mode 100755

it would be nice to remove mode changes from patch


> #ifdef MOZ_ENABLE_QTMOBILITY
>+#if (MOZ_PLATFORM_MAEMO > 5)

I think it is better to define in some header something like USE_ORIENTATION_SENSOR_FOR_WIDGET and define it when MOZ_ENABLE_QTMOBILITY && MOZ_PLATFORM_MAEMO > 5
so these defines would be simplier

> static const float GESTURES_BLOCK_MOUSE_FOR = 200;
> #ifdef MOZ_ENABLE_QTMOBILITY
>-#include <QtSensors/QOrientationSensor>
>+#include <QOrientationSensor>

don't we have -IQT_DIR/QtSensors in maemo5 build? why this change needed?


>-#endif
>+#else
>+#define HD_ZOOM_KEY_PROP "_HILDON_ZOOM_KEY_ATOM"
>+#endif //MOZ_PLATFORM_MAEMO == 6
>+#endif //MOZ_PLATFORM_MAEMO >= 5

Zoom key changes should come-up in separate patch

>+#if (MOZ_PLATFORM_MAEMO == 5)
>+        QObject::connect(QApplication::desktop(), SIGNAL(resized(int)), widget, SLOT(orientationChanged()));
>+#endif

hmm, I remember we have some mce orientation listener somewhere in toolkit/xre/nsNativeAppSupportUnix.cpp
can we reuse that for orientation changed signal?


>     }
> 
>     // synthesize a resize event if this isn't a toplevel
>-    if (mIsTopLevel || mListenForResizes) {
>+    if (!mIsTopLevel || mListenForResizes) {

Yep, comment describe opposite to what condition check does... but same stuff behave this way in GTK platform and kindof worked here.
is this change really participating fix for the orientation problem? is this really needed here? if it is just to fix comment vs impl conflict, then I would change comment in order to prevent breakage of existing working functionality...


>         nsIntRect rect(mBounds.x, mBounds.y, aWidth, aHeight);
>         nsEventStatus status;
>         DispatchResizeEvent(rect, status);

Comment 2

[Pali Rohár]

(In reply to Oleg Romashin (:romaxa) from comment #1)
> >+#if (MOZ_PLATFORM_MAEMO == 5)
> >+        QObject::connect(QApplication::desktop(), SIGNAL(resized(int)), widget, SLOT(orientationChanged()));
> >+#endif
> 
> hmm, I remember we have some mce orientation listener somewhere in
> toolkit/xre/nsNativeAppSupportUnix.cpp
> can we reuse that for orientation changed signal?

No, do not use MCE for rotation. Correct solution for rotation support is to listen for signal "window resized". Listening to MCE is bad idea because it break global rotation policy (like disable portrait mode). This problem has also integrated Maemo 5 MicroB browser.

Comment 3

[Steven Newbury]

I've broken out the patches on https://github.com/sjnewbury/fennec-qt-maemo5

I'll create new bugs under this one with them attached later today.

The LocationProvider has already been fixed now, so I'll drop that patch.

Comment 4

[Steven Newbury]

(In reply to Oleg Romashin (:romaxa) from comment #1)
> > static const float GESTURES_BLOCK_MOUSE_FOR = 200;
> > #ifdef MOZ_ENABLE_QTMOBILITY
> >-#include <QtSensors/QOrientationSensor>
> >+#include <QOrientationSensor>
> 
> don't we have -IQT_DIR/QtSensors in maemo5 build? why this change needed?
> 
This should be a separate patch, and isn't really complete. The issue is: when QTDIR is unset configure uses pkgconfig; if it sucssfully provides the flags, the prefix is incorrect.  If it fails it falls back to assuming it's somewhere in the search path prefixed with "QtSensors".

Comment 5

[Steven Newbury]

(In reply to Oleg Romashin (:romaxa) from comment #1)
> Comment on attachment 639052 [details] [diff] [review]

> 
> >     }
> > 
> >     // synthesize a resize event if this isn't a toplevel
> >-    if (mIsTopLevel || mListenForResizes) {
> >+    if (!mIsTopLevel || mListenForResizes) {
> 
> Yep, comment describe opposite to what condition check does... but same
> stuff behave this way in GTK platform and kindof worked here.
> is this change really participating fix for the orientation problem? is this
> really needed here? if it is just to fix comment vs impl conflict, then I
> would change comment in order to prevent breakage of existing working
> functionality...
After testing we can confirm this doesn't seem to make a difference.  We've dropped this change in recent builds and rotation has been working okay.

Comment 6

[BMO Automation]

Closing all opened bug in a graveyard component