Hello and welcome back to my weekly report.
As I said in the last report, I ended the week trying to fix the start menu closing on a second click. The realization that Windows doesn’t close the shell menus on mouse up, allowed me to simplify some of the logic, but the issue still resisted my attempts.
After some very long hours trying to figure out where the code went wrong, I realized the order of the events was actually wrong. This made me realize the problems I was having trying to get the code work were all due to the fact that the popup relays the mouse events to the window below itself, by using the PostMessage function. Since messages sent this way are not processed until the next time the message pump processes the rest of the queue, this mouse input message was always received after all the code that handles the menu bar logic for after a menu has closed would inevitably run before the mouse event was noticed.
In order to fix this, I had to resort to doing something that I consider a kludge: I intercepted the clicks to the menu bar done while a system popup is open, and I told the menu bar in advance, that it should remember to ignore the incoming mouse event. This works, but it has one “corner case” that is still unfixed: if you double-click a menu bar button, the popup opens, receives the double-click event, and closes after relaying the double-click event to the window below the mouse, which is the menu bar button. The result is that in this situation the menu bar ends up with the button in the “open” state, even though the popup has already closed, and the menu bar wrongly ignores the next mouse event (that didn’t happen as expected so it happens the next time you click).
I decided to leave that corner-case for later, and work on other issues instead. I looked at the TODO list, and decided to try adding some resource strings for hardcoded text in the shell, starting with the “(Empty)” text in empty menus. I created a resource for it, and implemented the loading code, and then when I was testing if it worked, I remembered that dummy item in the Favorites menu. Expecting an easy fix (or at least temporary workaround), I looked at the most fitting function and added a call to DeleteMenu with the menu item ID for this empty item. Annoyingly, this one function didn’t seem to be the place, as the menu item was still appearing. Testing in another place, I noticed two things: first, that the item was gone but there was some empty space left behind, and second that the menu handles were different in this function than the one in the previous one. The former, I hoped it would be a bug with the menu being deleted after it had been assigned to the popup, and for the latter I traced this handle to another call to SetMenu, which came through the shell view. Since this was a much better place to handle the removal, I moved the code there, but annoyingly this didn’t fix the space in the menu, meaning I was wrong in hoping it would be just a bug in the menu refreshing its size.
While I was still trying to debug this, I was told that recent builds from the branch were missing shortcuts. If those shortcuts had just been completely missing, maybe we could have blamed filesystem bugs – since the storage stack was being modified in trunk and we sync regularly – but the shortcuts weren’t missing, they were being created just fine, but in the wrong folders!
The code that creates these shortcuts is in the syssetup module, and starting form there we found one bug/hack after another. First, the syssetup code was using SHGetSpecialFolderPath, instead of the much more appropriate SHGetFolderPathAndSubDir, which was the perfect match for the use case. In using this function, it was losing the ability to tell the shell to create the shortcuts in the “Default User” folders.
After replacing the misused function with the better one, I noticed that although the start menu icons were now being created in the right place, the desktop icon for “Command Prompt” was still missing. A quick check in the profile folders revealed that this shortcut was still being created in the SYSTEM folder instead of the Default User one, and because of that, it was never copied to the Administrator user when the latter was created.
Looking at the code, it was apparent that the function was incomplete and using the wrong assumptions. Fixing took longer than it could have been, since I was missing the knowledge to do it right from the start. After a few bad tries, Giannis guided me toward the right set of functions for the task, all part of the UserEnv DLL.
Armed with this, I over-eagerly dumped the bad code and reimplemented it, but the subsequent failures told me I was doing it wrong. I had to recover some of the removed code, which I adapted and fused with my own code. The new version isn’t as different from the original as I originally thought it would be, since it is now clear that I misunderstood parts of the old code, but it is much cleaner and easy to verify.
That was the good news. The bad news is that some unrelated bug is preventing me from booting ReactOS in VMware after the second stage setup (the part with the GUI wizard), so I can't test it! Instead of booting into desktop, as soon as I choose an item of the FreeLdr menu, I get blink of blue and the VM immediately shuts down.
So being Friday afternoon already and me full of frustration, I launched Minecraft (in Windows 7, not ReactOS – I wish ;P) and planted some TNT around while laughing manically.
So I with a busy weekend ahead (Warlords of Draenor and a short trip to Barcelona), I hope to start next Monday with a working ros, and get better news for next week’s report.
See you around, and until next week.