Version: (using Devel) Installed from: Compiled sources OS: Linux If I click on the Device Notifier, leave the cursor to hover on one of the devices, and then (physically) unplug one of the removable devices, the list of devices will get corrupt. In particular, the first column on the list (device name, image and action name) completly disappears, and the second column (that may contain the Eject button) takes all the space. The list gets normal once another device is plugged in.
Created attachment 26512 [details] workaround/fix The corruptions seems to occur whenever a row is removed from the table (QStandardItemModel) while one of the items in the table is set as current (setCurrentIndex). Looks like a bug in Qt to me. This patch deselects any item prior to removing a device to prevent corruption from occuring, and also deselects it after inserting a new device, which removes a minor painting inconsistency in the same situation.
looks about right for row removal; but perhaps it makes sense to check if m_hoveredIndex is still valid before reseting it? for rowInsertion, it would be good to reset it to whatever the mouse is over?
Created attachment 26515 [details] fix corruption, recheck cursor Well yes, concerning removal, it doesn't need to be updated if there is nothing selected. I added a check. And yes, also it would be good to recheck the mouse position and update the highlight. I did that by calling the mouse event handler when something changes, but first deselecting the current selection (otherwise it wouldn't get updated as the row/column is the same but everything is shifted). I've used the rowsInserted method to do that when adding devices, but I had to use a signal from the data source to handle the "rowsRemoved" event (as there is no method to overload). So now it's all working properly - both the corruption is gone, and also the highlight is properly updated even if the cursor doesn't move.
Created attachment 26516 [details] fix corruption, recheck cursor forgot to shrink some lines into one...
SVN commit 839907 by aseigo: fix corruption on device plug/unplug while mouse hovered; based on patch by ambro BUG:167788 M +35 -1 notifierview.cpp M +6 -0 notifierview.h WebSVN link: http://websvn.kde.org/?view=rev&revision=839907
Created attachment 26528 [details] fix I think what you did there was not correct. Why did you remove the rowsAboutToBeRemoved? It is what fixes the corruption this bug is all about. This method is called before the actual removal, so this is the place where items have to be delesected (I tested it, bug not fixed). The modelRowsRemoved and rowsInserted methods I added are used only after the change, and all they do is refresh the highlight. Also, the disconnect statement should be removed from modelRowsRemoved, otherwise it would refresh properly on the first removal. This patch (diff of current svn): - adds rowsAboutToBeRemoved - changes rowsInserted to only invalidate the selection if there is one - changes modelRowsRemoved to never invalidate the selection (it is always already invalidated in rowsAboutToBeRemoved), and removes the disconnect. And I tested it, works in all aspects.
not fixed in svn see comment
It's still pretty much broken if used standalone on the desktop instead of the panel... I suggest you wait a little for me to rework things before changing anything.
Created attachment 26535 [details] fix This patch fixes all hover-related problems, both in panel and desktop mode. Correct ordering of operations is crucial; the principle is not to interfere with QTreeView's internal processing, in practice: - invalidate the selection even before QTreeView handles rowsAboutToBeRemoved, and before rows are inserted (it doesn't already have a slot for rowsAboutToBeInserted) - post (not invoke handler) a mouse move message after QTreeView handles rowsInserted/rowsRemoved I achieved the correct ordering of operations by overloading as many event handlers as possible; only rowsAboutToBeInserted did not exist, so I created a normal slot. Also, to correctly refresh the selection I had to cache the cursor position. That's becouse if the applet is on the desktop, the "global cursor position" is actually relative to the black box arround the applet (why??) so I can't just use QCursor::pos() which gives a true global position. Please, do not commit something you haven't tested thoroughly; it only causes confusion. If you think it can be done better, post your suggestions, so I can at least test it.
i did test it (obviously), including on the desktop, and there was no corruption visible while testing when plugging and unplugging devices. the reason i changed your patch was quite simple: connect/disconnect of the signal was done in the wrong place, testing seemed to indicate that the rowsAboutToBeRemoved implementation wasn't necessary (rowsAboutToBeRemoved was called, processing happened, then the rowsRemoved slot was called all before getting back to painting). the new patch is really ugly, but that's the fault of the view, not you. i'm actually surprised that there is painting happening here between rowsAboutToBeRemoved and actual row removal =/ > the "global cursor position" is > actually relative to the black box arround the applet yes, so that all operations in a widget can be done relative to the widget. this makes the common cases much easier; as for using QCursor::pos(), that will fail anyways unless you go through the view which you really don't want to do if at all possible since "the view" is not well defined enough for these kinds of cases due to multiple views being possible. p.s. plasma-devel is really a better place for these discussions
I cannot reproduce the weird corruption I was seeing anymore; you probably just noticed the highlight-not-refreshed problem. Anyway works perfectly with current svn.