Merge lp:~ken-vandine/ubuntu-system-settings/lp1438633 into lp:ubuntu-system-settings

[Ken VanDine]

Error checking for plugin instance and pageComponent

Error checking for plugin instance and pageComponent

[Sebastien Bacher (seb128)]

Thanks, but that looks like a workaround rather than a real fix, we should keep looking for the real bug there... also if we think that change is right, shouldn't we do the same for the regularitory info subscreen in the same source?

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1382
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/2052/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2206
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/322
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1938
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2204
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2204/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19583

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/2052/rebuild

review: Approve (continuous-integration)

[Ken VanDine (ken-vandine)]

> Thanks, but that looks like a workaround rather than a real fix, we should
> keep looking for the real bug there... also if we think that change is right,
> shouldn't we do the same for the regularitory info subscreen in the same
> source?

It's a fix because it prevents us from pushing an object on the PageStack that is null. It doesn't really fix why it was null before, I'd like to figure that out too, but so far no luck. It would be good to add the same protection for regulatory page too.

[Ken VanDine (ken-vandine)]

> Thanks, but that looks like a workaround rather than a real fix, we should
> keep looking for the real bug there... also if we think that change is right,
> shouldn't we do the same for the regularitory info subscreen in the same
> source?

The regulatory info page already sets it to a variable before pushing the page, so system-updates was the only one not setting that to a variable.

[Sebastien Bacher (seb128)]

ok, fair enough, I would still like us to understand the issue but the change looks safe and should improve things

review: Approve

=== modified file 'plugins/about/PageComponent.qml'
--- plugins/about/PageComponent.qml	2015-02-23 10:08:30 +0000
+++ plugins/about/PageComponent.qml	2015-04-13 19:12:26 +0000
@@ -196,8 +196,18 @@
                     objectName: "updateButton"
                     text: i18n.tr("Check for updates")
                     width: parent.width - units.gu(4)
-                    onClicked:
-                        pageStack.push(pluginManager.getByName("system-update").pageComponent)
+                    onClicked: {
+                        var upPlugin = pluginManager.getByName("system-update")
+                        if (upPlugin) {
+                            var updatePage = upPlugin.pageComponent
+                            if (updatePage)
+                                pageStack.push(updatePage)
+                            else
+                                console.warn("Failed to get system-update pageComponent")
+                        } else {
+                            console.warn("Failed to get system-update plugin instance")
+                        }
+                    }
                 }
                 showDivider: false
             }