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

Proposed by Ken VanDine
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 1005
Merged at revision: 1020
Proposed branch: lp:~ken-vandine/ubuntu-system-settings/lp1365646
Merge into: lp:ubuntu-system-settings
Diff against target: 26 lines (+3/-2)
2 files modified
plugins/system-update/PageComponent.qml (+1/-1)
plugins/system-update/system_update.cpp (+2/-1)
To merge this branch: bzr merge lp:~ken-vandine/ubuntu-system-settings/lp1365646
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Sebastien Bacher (community) Approve
Review via email: mp+234150@code.launchpad.net

Commit message

Don't call downloadUpdate for system updates that are already downloading (LP: #1365646)

Description of the change

Don't call downloadUpdate for system updates that are already downloading (LP: #1365646)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1005. By Ken VanDine

fixed the height calculation for the ListView, it was causing a crash onModelChanged

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks, could you explain why do you need to check the download mode there and what's the difference in the height computation/why the old value was leading to a segfault?

review: Needs Information
Revision history for this message
Ken VanDine (ken-vandine) wrote :

We don't want to tell system-image-dbus to download the system update if auto_download isn't enabled.

I'm not certain why using contentItem to get the height was causing the crash, it seemed to only happen when the system update was already downloaded, so got added to the model while the click updates were getting added. Note the system update gets prepended to the model, so maybe it has something to do with the reordering of the rows.

Revision history for this message
Sebastien Bacher (seb128) wrote :

thanks for the details, seems reasonable changes

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/system-update/PageComponent.qml'
--- plugins/system-update/PageComponent.qml 2014-09-09 16:39:35 +0000
+++ plugins/system-update/PageComponent.qml 2014-09-10 19:35:29 +0000
@@ -283,7 +283,7 @@
283 right: parent.right283 right: parent.right
284 }284 }
285 model: updateManager.model285 model: updateManager.model
286 height: contentItem.childrenRect.height286 height: childrenRect.height
287 interactive: false287 interactive: false
288288
289 delegate: ListItem.Subtitled {289 delegate: ListItem.Subtitled {
290290
=== modified file 'plugins/system-update/system_update.cpp'
--- plugins/system-update/system_update.cpp 2014-08-27 22:00:30 +0000
+++ plugins/system-update/system_update.cpp 2014-09-10 19:35:29 +0000
@@ -206,7 +206,8 @@
206 } else {206 } else {
207 Q_EMIT updateNotFound();207 Q_EMIT updateNotFound();
208 }208 }
209 if (downloading) {209
210 if (!downloading && (m_downloadMode == 0)) {
210 downloadUpdate();211 downloadUpdate();
211 update->setSelected(true);212 update->setSelected(true);
212 }213 }

Subscribers

People subscribed via source and target branches