From kde-panel-devel Wed Apr 15 08:58:12 2015 From: "Konrad Materka" Date: Wed, 15 Apr 2015 08:58:12 +0000 To: kde-panel-devel Subject: Re: Review Request 123350: Kickoff is not mounting removable devices and shows "Invalid URL" Message-Id: <20150415085812.4623.91287 () mimi ! kde ! org> X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=142908830729998 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1477099891915757133==" --===============1477099891915757133== Content-Type: multipart/alternative; boundary="===============6385261460231774862==" --===============6385261460231774862== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Kwi 13, 2015, 12:20 po południu, David Edmundson wrote: > > applets/kickoff/package/contents/ui/BaseView.qml, line 63 > > > > > > this change seems uneeded; we're using modelIndex fine in the old code, see line 40 in KickoffItem.qml in the old code. > > > > (unless maybe that didn't work ?) > > > > My understanding is setting the model to a QAIM, internally just creates a VDM anyway. It didn't work. modelIndex is used only if flag hasModelChildren is set to true. That can happen only for ApplicationView, as only there children exist. Also ApplicationView uses VisualDataModel explicitly, so method modelIndex is available. In SystemView model is set directly, VisualDataModel is not created implicitly (or even if it is it is not accessible). In other words listItem.ListView.view.model returns SystemModel object which is QAbstractProxyModel/QAbstractItemModel implementation. SystemModel does not have any slot that would be usefull in this case. Passing "model" variable is not an option. It would be a good idea as it contains all data related to current item but this is internal QML/Qt Quick class. In KDE 4 it was not a problem as it was easy to get QModelIndex from event (lines 860 - 878): https://projects.kde.org/projects/kde/kde-workspace/repository/entry/plasma/desktop/applets/kickoff/ui/launcher.cpp?rev=KDE%2F4.9 Patch is broken anyway, I didn't change FavoritesView to use VDM so not it fails with error "modelIndex is unknow property of FavoritiesModel" or something like that. Probably there are other issues. - Konrad ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123350/#review78891 ----------------------------------------------------------- On Kwi 15, 2015, 7:37 rano, Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123350/ > ----------------------------------------------------------- > > (Updated Kwi 15, 2015, 7:37 rano) > > > Review request for Plasma. > > > Bugs: 346002 > https://bugs.kde.org/show_bug.cgi?id=346002 > > > Repository: plasma-desktop > > > Description > ------- > > Patch by Konrad Materka > adress mounting of removable devices by kickoff > > > Diffs > ----- > > applets/kickoff/core/systemmodel.cpp 005cfb2 > applets/kickoff/package/contents/ui/BaseView.qml c74db68 > applets/kickoff/package/contents/ui/KickoffItem.qml f48d53b > > Diff: https://git.reviewboard.kde.org/r/123350/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > > --===============6385261460231774862== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123350/

On Kwiecień 13th, 2015, 12:20 po południu UTC, David Edmundson wrote:

applets/kickoff/package/contents/ui/BaseView.qml (Diff revision 1)
63
            model: VisualDataModel {

this change seems uneeded; we're using modelIndex fine in the old code, see line 40 in KickoffItem.qml in the old code.

(unless maybe that didn't work ?)

My understanding is setting the model to a QAIM, internally just creates a VDM anyway.

It didn't work. modelIndex is used only if flag hasModelChildren is set to true. That can happen only for ApplicationView, as only there children exist. Also ApplicationView uses VisualDataModel explicitly, so method modelIndex is available. In SystemView model is set directly, VisualDataModel is not created implicitly (or even if it is it is not accessible). In other words listItem.ListView.view.model returns SystemModel object which is QAbstractProxyModel/QAbstractItemModel implementation. SystemModel does not have any slot that would be usefull in this case. Passing "model" variable is not an option. It would be a good idea as it contains all data related to current item but this is internal QML/Qt Quick class.

In KDE 4 it was not a problem as it was easy to get QModelIndex from event (lines 860 - 878): https://projects.kde.org/projects/kde/kde-workspace/repository/entry/plasma/desktop/applets/kickoff/ui/launcher.cpp?rev=KDE%2F4.9

Patch is broken anyway, I didn't change FavoritesView to use VDM so not it fails with error "modelIndex is unknow property of FavoritiesModel" or something like that. Probably there are other issues.


- Konrad


On Kwiecień 15th, 2015, 7:37 rano UTC, Marco Martin wrote:

Review request for Plasma.
By Marco Martin.

Updated Kwi 15, 2015, 7:37 rano

Bugs: 346002
Repository: plasma-desktop

Description

Patch by Konrad Materka adress mounting of removable devices by kickoff

Diffs

  • applets/kickoff/core/systemmodel.cpp (005cfb2)
  • applets/kickoff/package/contents/ui/BaseView.qml (c74db68)
  • applets/kickoff/package/contents/ui/KickoffItem.qml (f48d53b)

View Diff

--===============6385261460231774862==-- --===============1477099891915757133== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============1477099891915757133==--