Unity 8

Merge lp://qastaging/~mterry/unity8/warn-on-xapp into lp://qastaging/unity8

warn-on-xapp
Merge into trunk

Proposed by Michael Terry on 2015-11-18

Status:	Work in progress
Proposed branch:	lp://qastaging/~mterry/unity8/warn-on-xapp
Merge into:	lp://qastaging/unity8
Prerequisite:	lp://qastaging/~mzanetti/unity8/modeswitchwarning
Diff against target:	752 lines (+258/-45) 27 files modified CMakeLists.txt (+2/-1) debian/control (+6/-6) plugins/Greeter/Unity/Launcher/CMakeLists.txt (+0/-1) plugins/Greeter/Unity/Launcher/launcheritem.cpp (+14/-0) plugins/Greeter/Unity/Launcher/launcheritem.h (+3/-0) plugins/Greeter/Unity/Launcher/launchermodelas.cpp (+10/-4) plugins/Unity/Launcher/CMakeLists.txt (+0/-1) plugins/Unity/Launcher/desktopfilehandler.cpp (+12/-0) plugins/Unity/Launcher/desktopfilehandler.h (+1/-0) plugins/Unity/Launcher/launcheritem.cpp (+14/-0) plugins/Unity/Launcher/launcheritem.h (+3/-0) plugins/Unity/Launcher/launchermodel.cpp (+30/-22) plugins/Unity/Launcher/launchermodel.h (+1/-1) qml/Components/Dialogs.qml (+26/-4) qml/Components/LegacyAppLaunchWarningDialog.qml (+51/-0) qml/Shell.qml (+9/-0) tests/mocks/Unity/Application/Application.qmltypes (+5/-0) tests/mocks/Unity/Application/ApplicationManager.cpp (+14/-0) tests/mocks/Unity/Application/ApplicationManager.h (+1/-0) tests/mocks/Unity/Launcher/CMakeLists.txt (+0/-2) tests/mocks/Unity/Launcher/MockLauncherItem.cpp (+13/-0) tests/mocks/Unity/Launcher/MockLauncherItem.h (+3/-0) tests/mocks/Unity/Launcher/MockLauncherModel.cpp (+2/-0) tests/plugins/Greeter/Unity/Launcher/CMakeLists.txt (+0/-2) tests/plugins/Unity/Launcher/CMakeLists.txt (+0/-1) tests/plugins/Unity/Launcher/launchermodeltest.cpp (+3/-0) tests/qmltests/tst_Shell.qml (+35/-0)
To merge this branch:	bzr merge lp://qastaging/~mterry/unity8/warn-on-xapp
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-01-13
Gerry Boland (community)			Needs Fixing on 2016-01-07
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2015-12-02
Albert Astals Cid (community)	merges fine		Abstain on 2015-11-30
Michael Zanetti (community)		2015-11-18	Needs Information on 2015-11-20
Review via email: mp+277915@code.qastaging.launchpad.net

Commit message

Prevent user from launching legacy xapps when in tablet or phone mode.

Description of the change

Prevent user from launching legacy xapps when in tablet or phone mode.

There are open design questions ("what should the text be?" and "should we make the legacy icons look different?") that I'm waiting on answers for. But this can be reviewed from a technical POV already.

Note that the LauncherItem changes to support isTouchApp aren't used yet. But they might be if Design wants a different look for them.

== Checklist ==

* Are there any related MPs required for this MP to build/function as expected? Please list.
https://code.launchpad.net/~mterry/unity-api/warn-on-xapp/+merge/277922
https://code.launchpad.net/~mterry/qtmir/warn-on-xapp/+merge/279172
https://code.launchpad.net/~mterry/ubuntu-app-launch/warn-on-xapp/+merge/278497

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
I'm on that team

* If you changed the UI, has there been a design review?
Not yet, working on it.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-11-19:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6743/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2015-11-20:

As discussed on IRC, this probably won't cut it. For instance it won't prevent the dash to launch the app. also anything else can do Qt.openUrlExternally(legacyApp).

I think the proper way to go would be to extend ApplicationManager to request permission for launching an app with the shell. Let's do a hangout with Gerry regarding this today.

review: Needs Information

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-11-20:

Text conflict in tests/qmltests/tst_Shell.qml
1 conflicts encountered.

review: Needs Fixing

lp://qastaging/~mterry/unity8/warn-on-xapp updated on 2015-11-20

2042. By Michael Terry on 2015-11-20: Merge from trunk

Revision history for this message

Michael Terry (mterry) wrote on 2015-11-20:

Fixed conflicts.

Revision history for this message

Albert Astals Cid (aacid) on 2015-11-20:

review: Abstain (merges fine)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-11-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6754/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-11-30:

Text conflict in CMakeLists.txt
1 conflicts encountered.

review: Needs Fixing

lp://qastaging/~mterry/unity8/warn-on-xapp updated on 2015-11-30

2043. By Michael Terry on 2015-11-30: Merge from trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-11-30:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6833/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) on 2015-11-30:

review: Abstain (merges fine)

lp://qastaging/~mterry/unity8/warn-on-xapp updated on 2015-12-01

2044. By Michael Terry on 2015-12-01: Update to use new approval API in ApplicationManager

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6859/rebuild

review: Needs Fixing (continuous-integration)

lp://qastaging/~mterry/unity8/warn-on-xapp updated on 2015-12-02

2045. By Michael Terry on 2015-12-02: Fix typo by actually specifying appId when docking

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-12-02:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/6870/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-12-14:

Text conflict in CMakeLists.txt
1 conflicts encountered.

Revision history for this message

Gerry Boland (gerboland) wrote on 2016-01-07:

=== modified file 'plugins/Greeter/Unity/Launcher/launchermodelas.cpp'
+ case RoleAlerting:
+ return item->alerting();
Unrelated to this MP.

=== modified file 'plugins/Unity/Launcher/desktopfilehandler.cpp'
Off topic (since I see it being done elsewhere in the file)

+bool DesktopFileHandler::isTouchApp() const
+{
+ if (isValid()) {
+ QSettings settings(m_filename, QSettings::IniFormat);
+ settings.setIniCodec("UTF-8");
+ settings.beginGroup(QStringLiteral("Desktop Entry"));
+ return settings.value(QStringLiteral("X-Ubuntu-Touch")).toBool(); // false for empty or "false"

Rest looks ok to me, but would prefer launcher owner have a look.
Creating the QSettings object and parse the desktop file for each property read isn't very efficient. Any idea why we don't just parse it once at DesktopFileHandler creation?

review: Needs Fixing

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2016-01-07:

> +bool DesktopFileHandler::isTouchApp() const
> +{
> + if (isValid()) {
> + QSettings settings(m_filename, QSettings::IniFormat);
> + settings.setIniCodec("UTF-8");
> + settings.beginGroup(QStringLiteral("Desktop Entry"));
> + return settings.value(QStringLiteral("X-Ubuntu-Touch")).toBool(); // false
> for empty or "false"
>
> Rest looks ok to me, but would prefer launcher owner have a look.
> Creating the QSettings object and parse the desktop file for each property
> read isn't very efficient. Any idea why we don't just parse it once at
> DesktopFileHandler creation?

From the QSettings docs: "Constructing and destroying a QSettings object is very fast."

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2016-01-07:

that said, parsing can be costly indeed. However, following the code around it, this is called only once when a launcher item is created. IMO we're ok here.

Revision history for this message

Michael Terry (mterry) wrote on 2016-01-07:

Regarding the QSettings, I didn't want to make any big changes because Lukas has a rewrite of that code in https://code.launchpad.net/~lukas-kde/unity8/desktopFileActions/+merge/276408 which converts to GKeyFile.

And as mzanetti said, it's one time.

Regarding the unrelated Role switch statement resortings, it's just a bit of cleanup. Everytime a property gets added to the application interface, they need to be added to a bunch of switch statements around the place. And currently, they are all in different orders, which makes it hard to tell if any are missing from one. For example, when adding RoleIsTouchApp for this MP, I noticed that launchermodelas.cpp was missing RoleRecent and RoleAlerting. So I added them here and sorted them to all be the same order, so it's less likely we'll miss future ones. So tangentially related, but not wholly unrelated.

Revision history for this message

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-01-13:

FAILED: Continuous integration, rev:2045
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/28/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-1-ci/28/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2016-01-25:

Text conflict in CMakeLists.txt
Text conflict in debian/control
2 conflicts

Unmerged revisions

2045. By Michael Terry on 2015-12-02: Fix typo by actually specifying appId when docking
2044. By Michael Terry on 2015-12-01: Update to use new approval API in ApplicationManager
2043. By Michael Terry on 2015-11-30: Merge from trunk
2042. By Michael Terry on 2015-11-20: Merge from trunk
2041. By Michael Terry on 2015-11-18: First pass at warning when launching a legacy app
2040. By Michael Terry on 2015-11-18: Merge in mzanetti's modeswitchwarning branch, we'll use a similar dialog

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes:

Michael Terry

Mohammad Rakibul Hasan

Unity Team