Bug - Fixed Missing relay script dropdown
- Thread starter Captain Scotch
- Start date
I think that is the root cause: visiting the account menu would change your detected topmenu style from "fancy" to "normal".
Changing from fancy to normal back to fancy would fix it - until the next time you visited the account menu.
Not trying to decorate awesomemenu.php?icons=1 is good, but I don't think we need to detect topmenu style from TopMenuDecorator. as per my experimental fix, any more. Fixing AccountRequest was key.
Changing from fancy to normal back to fancy would fix it - until the next time you visited the account menu.
Not trying to decorate awesomemenu.php?icons=1 is good, but I don't think we need to detect topmenu style from TopMenuDecorator. as per my experimental fix, any more. Fixing AccountRequest was key.
Looking at your movie again, I notice that every time we logged "We think topmenu style is NORMAL but it is actually FANCY", you had either:
1) Just visited the Account page - and changed to mobile mode, or whatever - and then did something which prompted KoL to refresh the awesomemenu.
2) Just visited awesomemenu.php?icons=1
I submitted a fix which did not "correct" menu style to "fancy" in case 2.
And now it is only case 1 which is screwing up.
So, yes: it was the Account page not recognizing the "fancy" radio button and setting TopMenuStyle to "normal" that was the root cause.
That fix is now in r27506.
(That still includes not even trying to decorate awesomemenu.php?icons=1)
If you are unable to reproduce any more "We think topmenu style is ..." errors, I will make a final (?) submit to remove the debugging code that does text searches in TopMenuDecorator to figure out what style you have and simply trusts that the style is correctly set by MoonPhaseRequest and AccountRequest, as appropriate. The awesomemenu.php?icons=1 fix will remain, since it need not (and can not) insert a script dropdown in the JSON.
1) Just visited the Account page - and changed to mobile mode, or whatever - and then did something which prompted KoL to refresh the awesomemenu.
2) Just visited awesomemenu.php?icons=1
I submitted a fix which did not "correct" menu style to "fancy" in case 2.
And now it is only case 1 which is screwing up.
So, yes: it was the Account page not recognizing the "fancy" radio button and setting TopMenuStyle to "normal" that was the root cause.
That fix is now in r27506.
(That still includes not even trying to decorate awesomemenu.php?icons=1)
If you are unable to reproduce any more "We think topmenu style is ..." errors, I will make a final (?) submit to remove the debugging code that does text searches in TopMenuDecorator to figure out what style you have and simply trusts that the style is correctly set by MoonPhaseRequest and AccountRequest, as appropriate. The awesomemenu.php?icons=1 fix will remain, since it need not (and can not) insert a script dropdown in the JSON.
I did some spot testing and couldn't make it reproduce. I tried a lot of things, including installing and removing scripts while I had that open. the @ckb menu worked really badly if it was on icon menu and we loaded his stuff, but it never told me it was detecting incorrectly or glitched. I don't know if this killed all the problem but you've definitely killed a problem.
Only observation I have is that the frameset has a fixed size in game.php and if we're going into a second row, we should expand the frame height so that our menu isn't hidden. This is in the enclosing page that loads awesomemenu, but if we can intercept that and expand the mainset size, that would be awesome. (this could be a new enhancement if it's worth pursuing).
When switching to the awesomemenu:
When Editing:
When you click to exit edit menu:
You can manually drag that fame separator down, but it would be nice if we caught that and added room for our menus if needed.
Only observation I have is that the frameset has a fixed size in game.php and if we're going into a second row, we should expand the frame height so that our menu isn't hidden. This is in the enclosing page that loads awesomemenu, but if we can intercept that and expand the mainset size, that would be awesome. (this could be a new enhancement if it's worth pursuing).
HTML:
<frameset id="mainset" rows="42,*"><frame name="menupane" src="topmenu.php" scrolling="no"><frame name="mainpane" src="main.php"></frameset>When switching to the awesomemenu:
When Editing:
When you click to exit edit menu:
You can manually drag that fame separator down, but it would be nice if we caught that and added room for our menus if needed.
With r27508, the reported issue should be fixed.
The bug was that every time you simply visited your account page (in particular, the Interface tab), even though you had the "Customizable Icons or Text" radio button selected - the "fancy" topmenu - we'd fail to recognize it and would set your topmenu style to "normal". And next time we needed to refresh your topmenu, it would put the relay script dropdown in a hidden and inaccessable place.
I disabled the debugging code that re-parsed the menu every time to "fix" the saved style.
The recent suggestion to enhance the actual HTML we insert to auto-resize the topmenu frame or what have you is a good idea - and if MCroft or somebody (not me) who understands CSS well wants to do it, cool - is out of scope for this particular bug report. I mean, I'm all for it, but that's a different bug/feature.
I'm marking this one "fixed".
The bug was that every time you simply visited your account page (in particular, the Interface tab), even though you had the "Customizable Icons or Text" radio button selected - the "fancy" topmenu - we'd fail to recognize it and would set your topmenu style to "normal". And next time we needed to refresh your topmenu, it would put the relay script dropdown in a hidden and inaccessable place.
I disabled the debugging code that re-parsed the menu every time to "fix" the saved style.
The recent suggestion to enhance the actual HTML we insert to auto-resize the topmenu frame or what have you is a good idea - and if MCroft or somebody (not me) who understands CSS well wants to do it, cool - is out of scope for this particular bug report. I mean, I'm all for it, but that's a different bug/feature.
I'm marking this one "fixed".