Bug 166481 - New Menu do not respect xdg files
Summary: New Menu do not respect xdg files
Status: RESOLVED FIXED
Alias: None
Product: plasma4
Classification: Plasma
Component: widget-kickoff (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Sebastian Sauer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-14 00:03 UTC by Nicolas L.
Modified: 2009-01-08 18:47 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Menu under kde3 (30.40 KB, image/png)
2008-07-14 00:04 UTC, Nicolas L.
Details
Menu under kde4 (79.34 KB, image/png)
2008-07-14 00:04 UTC, Nicolas L.
Details
xdg file from Mandriva (31.99 KB, application/octet-stream)
2008-07-14 13:06 UTC, Nicolas L.
Details
xdg file from Mandriva (31.83 KB, application/octet-stream)
2008-07-14 13:11 UTC, Nicolas L.
Details
patch that *may* fix it (1.91 KB, patch)
2008-08-05 17:30 UTC, Sebastian Sauer
Details
second try (2.64 KB, patch)
2008-08-06 00:43 UTC, Sebastian Sauer
Details
better (4.78 KB, patch)
2008-08-07 06:15 UTC, Sebastian Sauer
Details
typo-- (4.85 KB, patch)
2008-08-07 06:24 UTC, Sebastian Sauer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nicolas L. 2008-07-14 00:03:08 UTC
Version:            (using Devel)
Installed from:    Compiled sources

Hi,

looking current implementation of menu, i appears that there is some serious regressions on how it understand menu files .

We have asked ( thanks to /etc/xdg/menus/applications.menu ) to have some entries on the top of the menu, then the rest and at the bottom, the More section. This was well dispalyed on kde3 ( and on the other WM too ):

See attachment 1 [details]

But on kde4, there is no respect of this,  the More section is on the top, the wanted entries are not grouped, and there is no more separators.


See Attachment 2 [details]


Kmenuedit show the menu differently that on kickoff and classic whereas on kde3 the result was the same.
Comment 1 Nicolas L. 2008-07-14 00:04:01 UTC
Created attachment 26097 [details]
Menu under kde3
Comment 2 Nicolas L. 2008-07-14 00:04:33 UTC
Created attachment 26099 [details]
Menu under kde4
Comment 3 Aaron J. Seigo 2008-07-14 10:41:13 UTC
in kickoff, we put the folders first, and no, there aren't separators.

the entries set to be first should appear at the top though; right now i think we just alphabetize everything.

can you attach your modified /etc/xdg/menus/applications.menu so that we might have something to test with? thanks.
Comment 4 Nicolas L. 2008-07-14 13:06:13 UTC
Created attachment 26107 [details]
xdg file from Mandriva
Comment 5 Nicolas L. 2008-07-14 13:11:57 UTC
Created attachment 26108 [details]
xdg file from Mandriva
Comment 6 Nicolas L. 2008-07-14 13:12:25 UTC
please use the second one 
Comment 7 Nicolas L. 2008-07-18 22:33:59 UTC
i think that at list classical menu should follow xdg menu file as kde3 did
Comment 8 Sebastian Sauer 2008-08-02 04:10:48 UTC
the classic menu and kickoff share the same data-model what means, we could either;
1) introduce a sortproxymodel for kickoff to have the sorting done for kickoff but not for the classic menu or
2) kickoff and the classic menu just don't sort by themself but follow the specs here.

1) would probably be easier but also somewhat illogical imho since kmenuedit does not only allow to change the order (which has then no effect in kickoff) but also kmenuedit does not sort the items like kickoff does what is imho even more confusing.
2) would also need to touch the current logic we have within kickoff to show the description only if the prev/next item has the same name + imho to have it auto-sorted is a quit useful thing cause it does allow faster seek n' find of an item.
Comment 9 Sebastian Sauer 2008-08-02 04:13:25 UTC
since kmenuedit does not only allow to change the order => DOES allow
Comment 10 Nicolas L. 2008-08-04 08:08:04 UTC
i think the only  way is to follow .menu files if existing as we done before. For more coherency i think that classical menu and Kickoff should follow XDG spec by default.
Comment 11 Sebastian Sauer 2008-08-05 17:30:47 UTC
Created attachment 26653 [details]
patch that *may* fix it

Attached patch just
1) disables sorting for the app-menu and
2) extends the itemdelegate to don't depend on the sorting to determinate if
the title is unique and therefore it's needed to display the subtitle or not.

I fear the way how 2) is done is far from optimal since it means that we need
to loop through all items for each item what sounds like a good way to slow
down large menus :-/ Anyway, it's the first try to progress a bit :)

@Nicolas: does that work for you?
Comment 12 Sebastian Sauer 2008-08-05 17:35:33 UTC
re 2)
probably it would be better to have that logic in the model and just provide a "isUniqueTitle" role that delegate is able to ask for?
Comment 13 Nicolas L. 2008-08-05 22:26:38 UTC
i just tested and it didn't change at all the menu behaviour, this is still on the same order
Comment 14 Sebastian Sauer 2008-08-06 00:43:06 UTC
Created attachment 26676 [details]
second try

Now the patch does 1:1 the same thing we did in KDE3's service_mnu.cpp. But it
seems it doesn't have the expected effect here (though it may the case I did
something wrong on my testcases). I fear the prob is more down the stack within
kdelibs kservice/ksycoca-impl :-/
Comment 15 Nicolas L. 2008-08-06 16:32:07 UTC
this is a lot better.

On Office section it respect the .menu file but on :

Tools-> System Tools  we asked for a special placement thanks to the menu file and this is not respected.

In plus can we have some separators when this is asked on the menu file ?


Btw  thanks a lot for this begining of the fix :)
Comment 16 Sebastian Sauer 2008-08-07 06:15:25 UTC
Created attachment 26706 [details]
better

updated patch that addresses the performance-prob with large menus named in
comment #11. Else no new functionality.
Comment 17 Sebastian Sauer 2008-08-07 06:24:37 UTC
Created attachment 26707 [details]
typo--

rather stupid typo (time to catch some sleep ;)
Comment 18 Nicolas L. 2008-08-07 06:55:21 UTC
this is normal that there is no more a patch against workspace/libs/plasma/delegate.cpp ?
Comment 19 Sebastian Sauer 2008-08-08 00:06:47 UTC
y, it's correct. I just moved the changed logic in delegate to the model itself like "announced" in comment #12 ;)
Comment 20 Nicolas L. 2008-08-15 23:33:20 UTC
this is better on i586 but on x86_64 this doesn't work at all.
btw we still miss separators and on my i586 xdg is only respected on the first section of the menu but on subsection this is not
Comment 21 Nicolas L. 2009-01-08 07:34:22 UTC
ping for this important regression in menus
Comment 22 Aaron J. Seigo 2009-01-08 18:18:33 UTC
"this is better on i586 but on x86_64 this doesn't work at all."

erm, but there's no platform specific code in Sebastian's patch whatsoever.

"we still miss separators"

sure, that's a TODO in there.

"is only respected on the first section of the menu but on subsection this is not"

subsection, meaning submenus? e.g. the second level of menus?

the patch doesn't even apply to trunk anymore. i'm currently working on that.



as a side commentary, i think that providing manual ordering of items in this fashion is about as sensible as allowing the user to say "i'd like this item at pixel 100, and that one at pixel 110". it's quite frankly stupid. unfortunately, in typical xdg fashion, i don't think much real design thought went into it and it was just "hey, people should be able to order the menu" without asking "why?" or "what are they really trying to achieve?" or "how might this interact with other valid features?" ... nope, just slap-dash design-then-code.

i realize that we "have" to support it now at this point, but it's so very dissapointing to have such legacy dragging on things. as a heads up: if/when we get around to providing an improvement interace to applications, i do not expect we will care much for the xdg spec in designing it.

this is, however, precisely the kind of work that takes out all the joy of working on "desktop linux". "yay, more drudge work for a design i don't agree with made by people i don't feel were properly equipped to make such a design for a use case i'll probably never personally care about."
Comment 23 Aaron J. Seigo 2009-01-08 18:42:40 UTC
SVN commit 907746 by aseigo:

listen to the menu files as per the xdg spec for these things.
based on a patch by Sebastian Sauer.
BUG:166481


 M  +102 -56   applicationmodel.cpp  
 M  +7 -1      applicationmodel.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=907746
Comment 24 Aaron J. Seigo 2009-01-08 18:47:19 UTC
SVN commit 907750 by aseigo:

backport of fix for bug 166481
CCBUG:166481


 M  +102 -56   applicationmodel.cpp  
 M  +7 -1      applicationmodel.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=907750