webbrowser-app

Merge lp:~gerboland/webbrowser-app/formFactor-support into lp:webbrowser-app

  1. formFactor-support
  2. Merge into trunk
Proposed by Gerry Boland
Status: Needs review
Proposed branch: lp:~gerboland/webbrowser-app/formFactor-support
Merge into: lp:webbrowser-app
Diff against target: 298 lines (+99/-67)
5 files modified
src/Ubuntu/Components/Extras/Browser/CMakeLists.txt (+2/-0)
src/Ubuntu/Web/CMakeLists.txt (+2/-0)
src/Ubuntu/Web/UbuntuWebContext.qml (+3/-2)
src/Ubuntu/Web/UserAgent02.qml (+2/-6)
src/Ubuntu/Web/plugin.cpp (+90/-59)
To merge this branch: bzr merge lp:~gerboland/webbrowser-app/formFactor-support
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Ubuntu Phablet Team Pending
Review via email: mp+286790@code.launchpad.net

Commit message

Use formFactor property supplied by ubuntumirclient QPA where possible. Remove smallScreen heuristics

DO NOT LAND YET. This is (a) something for MWC, and (b) a request for comments if this approach looks reasonable to you

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

I’m not seeing any formFactor property defined on UserAgent02, so I doubt the onFormFactorChanged handler in UbuntuWebContext.qml would work.

Note that I’m currently working on a branch that removes the 'formFactor' property from the Ubuntu.Web plugin’s context object¹, so it’s kind of ironical that you’re adding it back in another branch. I really don’t think the default user agent (and corresponding overrides) should be tied to the "form factor" (whatever that means in a world of convergence). How does mir determine whether a device is a phone or a desktop, or a tablet? Is that a solid API that we are going to support and publicize?

Even if that was a sensible thing to do, we most probably don’t want to get a mobile UX on a tablet. Well, at least not a 10" tablet. On a 7" tablet maybe. See where the screenDiagonal property comes from?

As a side note, the dependency on Qt5Gui_PRIVATE is not very welcome, given that we’ve been steadily working towards entirely removing the dependencies on private Qt packages (ask Mirv about it).

¹ https://code.launchpad.net/~osomon/webbrowser-app/remove-formFactor/+merge/285539

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1358
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2626/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6587/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1379/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1379/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1379/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6598/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2626/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Gerry Boland (gerboland) wrote :

> I’m not seeing any formFactor property defined on UserAgent02, so I doubt the
> onFormFactorChanged handler in UbuntuWebContext.qml would work.

Code untested. Was struggling to cross compile & deploy, sbuild unhappy with me today.

> Note that I’m currently working on a branch that removes the 'formFactor'
> property from the Ubuntu.Web plugin’s context object¹, so it’s kind of
> ironical that you’re adding it back in another branch. I really don’t think
> the default user agent (and corresponding overrides) should be tied to the
> "form factor" (whatever that means in a world of convergence). How does mir
> determine whether a device is a phone or a desktop, or a tablet? Is that a
> solid API that we are going to support and publicize?

I obviously have no idea of your plans, this was just an attempt at replacing your screen heuristic with something more concrete.

Form Factor will be a hint to the application to tell it what mode the shell is in, on each screen. Mir supports it, shell will do its best to set it correctly. I would hope the average app author has no need for such a property, any form factor specific behaviour would be encapsulated in the UITK.

> Even if that was a sensible thing to do, we most probably don’t want to get a
> mobile UX on a tablet. Well, at least not a 10" tablet. On a 7" tablet maybe.
> See where the screenDiagonal property comes from?

Is your call. The goal of this was to indicate that you can learn if the app is running on a tablet or a desktop UI at the time.

> As a side note, the dependency on Qt5Gui_PRIVATE is not very welcome, given
> that we’ve been steadily working towards entirely removing the dependencies on
> private Qt packages (ask Mirv about it).

Qt doesn't have a nice way to export it to apps yet, hence having to resort to private Qt headers. As I mentioned in a comment, a better solution is for the UITK to do that for you. This isn't the final solution.
Thanks for the comments!

Reply
lp:~gerboland/webbrowser-app/formFactor-support updated
1359. By Gerry Boland

formFactor not in scope, fix

Revision history for this message
Olivier Tilloy (osomon) wrote :

> > Even if that was a sensible thing to do, we most probably don’t want
> > to get a mobile UX on a tablet. Well, at least not a 10" tablet. On
> > a 7" tablet maybe. See where the screenDiagonal property comes from?
>
> Is your call. The goal of this was to indicate that you can learn if
> the app is running on a tablet or a desktop UI at the time.

Thanks, and that’s useful, but I’m not convinced that (or at least that *alone*) is enough to decide what the default UA string should be. I have started a conversation with design about that, we need to come to a consensus. Until that happens, I’d rather keep the current implementation based on screen size (I’m not against refining it though).

Reply
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

> > I’m not seeing any formFactor property defined on UserAgent02, so I doubt
> the
> > onFormFactorChanged handler in UbuntuWebContext.qml would work.
>
> Code untested. Was struggling to cross compile & deploy, sbuild unhappy with
> me today.
>
> > Note that I’m currently working on a branch that removes the 'formFactor'
> > property from the Ubuntu.Web plugin’s context object¹, so it’s kind of
> > ironical that you’re adding it back in another branch. I really don’t think
> > the default user agent (and corresponding overrides) should be tied to the
> > "form factor" (whatever that means in a world of convergence). How does mir
> > determine whether a device is a phone or a desktop, or a tablet? Is that a
> > solid API that we are going to support and publicize?
>
> I obviously have no idea of your plans, this was just an attempt at replacing
> your screen heuristic with something more concrete.
>
> Form Factor will be a hint to the application to tell it what mode the shell
> is in, on each screen. Mir supports it, shell will do its best to set it
> correctly. I would hope the average app author has no need for such a
> property, any form factor specific behaviour would be encapsulated in the
> UITK.
>

It sounds like this might be useful for bug 1545088, bug 1547145 and bug 1547102. How does the shell determine what the hint is?

> > Even if that was a sensible thing to do, we most probably don’t want to get
> a
> > mobile UX on a tablet. Well, at least not a 10" tablet. On a 7" tablet
> maybe.
> > See where the screenDiagonal property comes from?
>
> Is your call. The goal of this was to indicate that you can learn if the app
> is running on a tablet or a desktop UI at the time.
>
> > As a side note, the dependency on Qt5Gui_PRIVATE is not very welcome, given
> > that we’ve been steadily working towards entirely removing the dependencies
> on
> > private Qt packages (ask Mirv about it).
>
> Qt doesn't have a nice way to export it to apps yet, hence having to resort to
> private Qt headers. As I mentioned in a comment, a better solution is for the
> UITK to do that for you. This isn't the final solution.
> Thanks for the comments!

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1359
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2627/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/6588
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1380
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1380
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1380/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1380
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/5011
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6599
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/6599/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27786

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2627/rebuild

review: Needs Fixing (continuous-integration)
Reply

Unmerged revisions

1359. By Gerry Boland

formFactor not in scope, fix

1358. By Gerry Boland

Use formFactor property supplied by ubuntumirclient QPA where possible. Remove smallScreen heuristics

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Ubuntu/Components/Extras/Browser/CMakeLists.txt'
2--- src/Ubuntu/Components/Extras/Browser/CMakeLists.txt 2015-06-22 10:29:20 +0000
3+++ src/Ubuntu/Components/Extras/Browser/CMakeLists.txt 2016-02-22 12:16:23 +0000
4@@ -10,6 +10,8 @@
5
6 set(PLUGIN_SRC plugin.cpp)
7
8+include_directories(${Qt5Gui_PRIVATE_INCLUDE_DIRS})
9+
10 add_library(${PLUGIN} MODULE ${PLUGIN_SRC})
11 target_link_libraries(${PLUGIN}
12 Qt5::Core
13
14=== modified file 'src/Ubuntu/Web/CMakeLists.txt'
15--- src/Ubuntu/Web/CMakeLists.txt 2015-08-25 13:56:58 +0000
16+++ src/Ubuntu/Web/CMakeLists.txt 2016-02-22 12:16:23 +0000
17@@ -10,6 +10,8 @@
18
19 set(PLUGIN_SRC plugin.cpp)
20
21+include_directories(${Qt5Gui_PRIVATE_INCLUDE_DIRS})
22+
23 add_library(${PLUGIN} MODULE ${PLUGIN_SRC})
24 target_link_libraries(${PLUGIN}
25 Qt5::Core
26
27=== modified file 'src/Ubuntu/Web/UbuntuWebContext.qml'
28--- src/Ubuntu/Web/UbuntuWebContext.qml 2015-12-11 10:30:42 +0000
29+++ src/Ubuntu/Web/UbuntuWebContext.qml 2016-02-22 12:16:23 +0000
30@@ -70,13 +70,14 @@
31 ]
32
33 property QtObject __ua: UserAgent02 {
34- onSmallScreenChanged: reloadOverrides()
35+ readonly property string formFactor: formFactor
36+ onFormFactorChanged: reloadOverrides()
37 Component.onCompleted: reloadOverrides()
38
39 property string _target: ""
40
41 function reloadOverrides() {
42- var target = smallScreen ? "mobile" : "desktop"
43+ var target = formFactor === "MOBILE" ? "mobile" : "desktop"
44 if (target == _target) return
45 _target = target
46 var script = "ua-overrides-%1.js".arg(target)
47
48=== modified file 'src/Ubuntu/Web/UserAgent02.qml'
49--- src/Ubuntu/Web/UserAgent02.qml 2015-12-10 10:43:04 +0000
50+++ src/Ubuntu/Web/UserAgent02.qml 2016-02-22 12:16:23 +0000
51@@ -28,10 +28,6 @@
52 */
53
54 QtObject {
55- // Empirical value: screens smaller than 19cm are considered small enough that a
56- // mobile UA string is used, screens bigger than that will get desktop content.
57- readonly property bool smallScreen: screenDiagonal < 190
58-
59 // %1: Ubuntu version, e.g. "14.04"
60 // %2: optional token to specify further attributes of the platform, e.g. "like Android"
61 // %3: optional hardware ID token
62@@ -50,7 +46,7 @@
63 // FIXME: compute at build time (using lsb_release)
64 readonly property string _ubuntuVersion: "14.04"
65
66- readonly property string _attributes: smallScreen ? "like Android 4.4" : ""
67+ readonly property string _attributes: formFactor === "MOBILE" ? "like Android 4.4" : ""
68
69 readonly property string _hardwareID: ""
70
71@@ -63,7 +59,7 @@
72 // every time we rebase on a newer chromium.
73 readonly property string _chromiumVersion: "35.0.1870.2"
74
75- readonly property string _formFactor: smallScreen ? "Mobile" : ""
76+ readonly property string _formFactor: formFactor === "MOBILE" ? "Mobile" : ""
77
78 readonly property string _more: ""
79
80
81=== modified file 'src/Ubuntu/Web/plugin.cpp'
82--- src/Ubuntu/Web/plugin.cpp 2015-12-10 22:01:05 +0000
83+++ src/Ubuntu/Web/plugin.cpp 2016-02-22 12:16:23 +0000
84@@ -34,14 +34,15 @@
85 #include <QtQml>
86 #include <QtQml/QQmlInfo>
87
88+#include <QtGui/qpa/qplatformnativeinterface.h>
89+
90 class UbuntuWebPluginContext : public QObject
91 {
92 Q_OBJECT
93
94 Q_PROPERTY(QString cacheLocation READ cacheLocation NOTIFY cacheLocationChanged)
95 Q_PROPERTY(QString dataLocation READ dataLocation NOTIFY dataLocationChanged)
96- Q_PROPERTY(QString formFactor READ formFactor CONSTANT)
97- Q_PROPERTY(qreal screenDiagonal READ screenDiagonal NOTIFY screenDiagonalChanged)
98+ Q_PROPERTY(QString formFactor READ formFactor NOTIFY formFactorChanged)
99 Q_PROPERTY(int cacheSizeHint READ cacheSizeHint NOTIFY cacheSizeHintChanged)
100 Q_PROPERTY(QString webviewDevtoolsDebugHost READ devtoolsHost CONSTANT)
101 Q_PROPERTY(int webviewDevtoolsDebugPort READ devtoolsPort CONSTANT)
102@@ -53,7 +54,6 @@
103 QString cacheLocation() const;
104 QString dataLocation() const;
105 QString formFactor();
106- qreal screenDiagonal() const;
107 int cacheSizeHint() const;
108 QString devtoolsHost();
109 int devtoolsPort();
110@@ -62,15 +62,15 @@
111 Q_SIGNALS:
112 void cacheLocationChanged() const;
113 void dataLocationChanged() const;
114- void screenDiagonalChanged() const;
115+ void formFactorChanged() const;
116 void cacheSizeHintChanged() const;
117
118 private Q_SLOTS:
119- void onFocusWindowChanged(QWindow* window);
120- void updateScreen();
121+ void onWindowPropertyChanged(QPlatformWindow* window, const QString &property);
122
123 private:
124- qreal m_screenDiagonal; // in millimeters
125+ void prepareFormFactor();
126+ QString readFormFactorFromPlatform(QPlatformWindow *window);
127 QString m_formFactor;
128 QString m_devtoolsHost;
129 int m_devtoolsPort;
130@@ -80,30 +80,57 @@
131
132 UbuntuWebPluginContext::UbuntuWebPluginContext(QObject* parent)
133 : QObject(parent)
134- , m_screenDiagonal(0)
135 , m_devtoolsPort(-2)
136 , m_hostMappingRulesQueried(false)
137 {
138 connect(qApp, SIGNAL(applicationNameChanged()), SIGNAL(cacheLocationChanged()));
139 connect(qApp, SIGNAL(applicationNameChanged()), SIGNAL(dataLocationChanged()));
140 connect(qApp, SIGNAL(applicationNameChanged()), SIGNAL(cacheSizeHintChanged()));
141- updateScreen();
142- connect(qApp, SIGNAL(focusWindowChanged(QWindow*)), SLOT(onFocusWindowChanged(QWindow*)));
143+ prepareFormFactor();
144 }
145
146-void UbuntuWebPluginContext::updateScreen()
147+namespace {
148+ // This implementation only considers two possible form factors: desktop,
149+ // and mobile (which includes phones and tablets).
150+ // XXX: do we need to consider other form factors, such as tablet?
151+ const char* DESKTOP = "desktop";
152+ const char* MOBILE = "mobile";
153+} // anonymous namespace
154+
155+void UbuntuWebPluginContext::prepareFormFactor()
156 {
157- QWindow* window = qApp->focusWindow();
158- if (window) {
159- QScreen* screen = window->screen();
160- if (screen) {
161- QSizeF size = screen->physicalSize();
162- qreal diagonal = qSqrt(size.width() * size.width() + size.height() * size.height());
163- if (diagonal != m_screenDiagonal) {
164- m_screenDiagonal = diagonal;
165- Q_EMIT screenDiagonalChanged();
166+ // The "DESKTOP_MODE" environment variable can be used to force the form
167+ // factor to desktop, when set to any valid value other than 0.
168+ const char* DESKTOP_MODE_ENV_VAR = "DESKTOP_MODE";
169+ if (qEnvironmentVariableIsSet(DESKTOP_MODE_ENV_VAR)) {
170+ QByteArray stringValue = qgetenv(DESKTOP_MODE_ENV_VAR);
171+ bool ok = false;
172+ int value = stringValue.toInt(&ok);
173+ if (ok) {
174+ m_formFactor = (value == 0) ? MOBILE : DESKTOP;
175+ return;
176+ }
177+ }
178+
179+ // If using QtUbuntu, try fetch formFactor property, falling back to assumption of mobile if that fails
180+ QString platform = QGuiApplication::platformName();
181+ if (platform != "ubuntumirclient") {
182+ m_formFactor = DESKTOP;
183+ } else { // We will get notifications of a "formFactor" property changing from the QPA for all windows
184+ auto nativeInterface = qGuiApp->platformNativeInterface();
185+ connect(nativeInterface,
186+ SIGNAL(windowPropertyChanged(QPlatformWindow*, const QString&)),
187+ SLOT(onWindowPropertyChanged(QPlatformWindow*, const QString&)));
188+
189+ auto window = qGuiApp->allWindows().first(); // FIXME: context property not suitable for per-window formFactor
190+ if (window) {
191+ auto formFactor = readFormFactorFromPlatform(window->handle());
192+ if (!formFactor.isEmpty()) {
193+ m_formFactor = formFactor;
194+ return;
195 }
196 }
197+ m_formFactor = MOBILE; // fallback position
198 }
199 }
200
201@@ -131,43 +158,9 @@
202
203 QString UbuntuWebPluginContext::formFactor()
204 {
205- if (m_formFactor.isEmpty()) {
206- // This implementation only considers two possible form factors: desktop,
207- // and mobile (which includes phones and tablets).
208- // XXX: do we need to consider other form factors, such as tablet?
209- const char* DESKTOP = "desktop";
210- const char* MOBILE = "mobile";
211-
212- // The "DESKTOP_MODE" environment variable can be used to force the form
213- // factor to desktop, when set to any valid value other than 0.
214- const char* DESKTOP_MODE_ENV_VAR = "DESKTOP_MODE";
215- if (qEnvironmentVariableIsSet(DESKTOP_MODE_ENV_VAR)) {
216- QByteArray stringValue = qgetenv(DESKTOP_MODE_ENV_VAR);
217- bool ok = false;
218- int value = stringValue.toInt(&ok);
219- if (ok) {
220- m_formFactor = (value == 0) ? MOBILE : DESKTOP;
221- return m_formFactor;
222- }
223- }
224-
225- // XXX: Assume that QtUbuntu means mobile, which is currently the case,
226- // but may not remain true forever.
227- QString platform = QGuiApplication::platformName();
228- if ((platform == "ubuntu") || (platform == "ubuntumirclient")) {
229- m_formFactor = MOBILE;
230- } else {
231- m_formFactor = DESKTOP;
232- }
233- }
234 return m_formFactor;
235 }
236
237-qreal UbuntuWebPluginContext::screenDiagonal() const
238-{
239- return m_screenDiagonal;
240-}
241-
242 int UbuntuWebPluginContext::cacheSizeHint() const
243 {
244 if (QCoreApplication::applicationName() == "webbrowser-app") {
245@@ -242,11 +235,49 @@
246 return m_devtoolsPort;
247 }
248
249-void UbuntuWebPluginContext::onFocusWindowChanged(QWindow* window)
250-{
251- updateScreen();
252- if (window) {
253- connect(window, SIGNAL(screenChanged(QScreen*)), SLOT(updateScreen()));
254+void UbuntuWebPluginContext::onWindowPropertyChanged(QPlatformWindow *window, const QString &property)
255+{
256+ if (property != QStringLiteral("formFactor")) {
257+ return;
258+ }
259+
260+ auto formFactor = readFormFactorFromPlatform(window);
261+ if (!formFactor.isEmpty()) {
262+ m_formFactor = formFactor;
263+ Q_EMIT formFactorChanged();
264+ }
265+}
266+
267+// FIXME: this should be a per-window/screen property nicely wrapped by the UITK
268+QString UbuntuWebPluginContext::readFormFactorFromPlatform(QPlatformWindow *window)
269+{
270+ auto nativeInterface = qGuiApp->platformNativeInterface();
271+ QVariant formFactorVal = nativeInterface->windowProperty(window, QStringLiteral("formFactor"));
272+ if (!formFactorVal.isValid()) {
273+ return QString();
274+ }
275+
276+ // taken from MirFormFactor enum in mirclient/common.h
277+ typedef enum MirFormFactor
278+ {
279+ mir_form_factor_unknown,
280+ mir_form_factor_phone,
281+ mir_form_factor_tablet,
282+ mir_form_factor_monitor,
283+ mir_form_factor_tv,
284+ mir_form_factor_projector,
285+ } MirFormFactor;
286+
287+ bool ok;
288+ auto formFactor = static_cast<MirFormFactor>(formFactorVal.toInt(&ok));
289+ if (!ok) {
290+ return QString();
291+ }
292+
293+ if (formFactor == mir_form_factor_phone || formFactor == mir_form_factor_tablet) {
294+ return MOBILE;
295+ } else {
296+ return DESKTOP;
297 }
298 }
299

Subscribers

People subscribed via source and target branches

to status/vote changes: