48774 - sortStaticsLast property doesn't work

Status: RESOLVED INVALID

(Core :: XUL, defect, P3)

Description

[Rann Bar-On]

the line <menupopup sortStaticsLast="true"> in navigatorOverlay.xul makes a mess
of the View -> Languages and Web Content submenu. If it's changed to simply
<menupopup>, it works, but doesn't sort, so the menu looks upside down.

To reproduce:
1. look at view -> languages and web content submenu
2. open navigatorOverlay.xul
3. find <menupopup sortStaticsLast="true"> and change to <menupopup>
4. restart mozilla and look at the submenu again

I think the sortStaticsLast routine in nsXULSortService.cpp is broken

Comment 1

[John Morrison]

sortStaticsLast is rjc's baby ...

Comment 2

[Robert John Churchill]

This works fine on Mac (native menus), but seems to sometimes confuse dynamic 
menus on both Windows & Linux.  David, any idea?

Comment 3

[Peter Trudelle]

Dave sez he doesn't know anything about this, and to give it back to you. ->rjc

Comment 4

[Robert John Churchill]

"sortStaticsList" affects where the XUL Sort service inserts nodes.  Again, this 
works on Mac with native menus, but not with dynamic menus on Windows or Linux. 
 Not sure what I can do about this, smells like a problem with XUL.  David, any 
help you can provide would be appreciated.  :^)

Comment 5

[John Morrison]

*** Bug 51094 has been marked as a duplicate of this bug. ***

Comment 6

[John Morrison]

Nom. nsbeta3 as this apparently blocks two other bugs, but we need to find 
the right owner for this.

Comment 7

[Peter Trudelle]

nsbeta3+ to either fix the sortStatics code, or disable this feature, whichever
is the lesser amount of work. I see that 48646 has a workaround, and one may
need to live with the ordering as is for NS6 (bug 47343).

Comment 8

[Peter Trudelle]

really plussing. P3 for M18

Comment 9

[David Hyatt]

Reassigning to pink.  Something is going wrong with the way RDF is notifying the 
XP menus.

Comment 10

[Mike Pinkerton (not reading bugmail)]

*puuunt*

Comment 11

[Robert John Churchill]

Kickback time.

Comment 12

[Peter Trudelle]

nsbeta3-/future, Should workaround by putting the statics first, which is
probably better from a usability standpoint anyway, since there is less chance
of extraneous submenus popping up as you traverse to the static menus.

Comment 13

[Frank Tang]

please reconsider fixing this. 47343 is depend on this. Also, Mac is ok. Only 
window and Linux have this problem.

Look at 47343 for the new charsetOverlay.xul we proposed. If you use that xul, 
it show correctly on Mac but not window.

Comment 14

[Peter Trudelle]

Do we have any clue about this?  Would fixing it require some effort from all
three (pink/hyatt/rjc) of you?  I'm trying to determine the cost/risk of fixing
this vs having I18N use a workaround for bug 47343.  I figured that
sortStaticsLast was an obscure feature, but is is really only used in this one
place?

Comment 15

[Peter Trudelle]

Pink says fixing this on our end would be a research project, involving
interactions between RDF and XPMenus.  We aren't familiar with the code, so this
could take a long time and be risky.  Since *nobody* currently uses this feature
anywhere in the product, we don't think it is is worth the time/risk.  

Comment 16

[Robert John Churchill]

Research problem?  Yeah, OK, whatever.  This has virtually nothing to do with 
RDF actually.  If you look here:

http://lxr.mozilla.org/seamonkey/source/rdf/content/src/nsXULSortService.cpp#215
1

you'll see that all that's really happening is that 
<nsIContent>::InsertChildAt() is used to insert nodes in various places... and 
that XP menus (but not Mac native menus) seem to get confused.

Comment 17

[Mike Pinkerton (not reading bugmail)]

i know, i know...but the investigation into that part of the menu code (of which 
i'm not familiar) will take some time for _me_. maybe you know it better, but I 
don't see you volunteering.

Comment 18

[Robert John Churchill]

I'm not volunteering because I don't know anything about the menu code. If you
don't either, then say so and perhaps pass this bug over to whoever IS familiar
with the menu code?  Since others have asked for this bug to be fixed...

Comment 19

[Peter Trudelle]

'Others asking' is not the primary criteria we're using for triage. We think
this can be trivially worked around with negligable effect on product quality,
which will allow us to fix other, more serious bugs. 

Comment 20

[Peter Trudelle]

nsbeta3-

Comment 21

[Mike Pinkerton (not reading bugmail)]

new owner

Comment 22

[Chase Tingley]

Folding this in to the rewrite I've got going in bug 105783.

Comment 23

[Chase Tingley]

Umm, so what exactly is going on here?

sortStatisLast has been removed from the View -> Languages and Web Contet menu.
The menu looks like this:
+--------------+
| Download More|
|--------------|
| English(US)  |
| US Region    |
+--------------+

If I add the sortStaticsLast bit back, i see this:

+--------------+
| English(US)  |
| US Region    |
| Download More|
|--------------|
+--------------+

Is this the same problem that was originally described here?  In this case,
sortStaticsLast is actually working -- reversing "Download More" and the
splitter would require changing the sort weights somehow.  Perhaps a special
case needs to be made for splitters as the are in the DoSort() code when
inbetweenSeparatorSort is in effect.  Or perhaps I'm just confused.

Comment 24

[Chase Tingley]

Attachment: testcase - menu.xul

Comment 25

[Chase Tingley]

Attachment: testcase - menu.rdf

Comment 26

[Chase Tingley]

There's an off chance that the broken index code in the sort service was causing
dynamic content to be sorted after static when it wasn't supposed to, (although
I never saw this problem).

Please take a look at the testcase; it should present several dynamic
alternatives pulled from RDF followed by a separator and an "other" choice.

Comment 27

[Mihai Morar, (:MihaiMorar)]

Menu section got several updates since FF 4.0. None of those 2 submenus (languages and Web Content are no longer displayed in View section.