aceman: "RDF" was in pre-existing names, but I can go ahead and remove it in the refactoring patch (part 1). Thanks for the heads up on that. To respond to your other comments, rather than just copy the code and modify it, I'm following DRY principles to avoid code duplication and maintenance overhead. As you can see in the patch there is a lot of shared code between the two cases but also significant differences in the menus that need different handling. For example, <menupopup>s are children of a <menu> leading to a deeply nested structure but <panelview>s are siblings, a flat structure. You can append menu items directly to a <menupopup> but a <panelview> has a <vbox> child where menu items need to be appended. Etc. (Search for "this.localName ==" (menupopup or panelview) to see where there's branching and what the differences are.) While the code likely can be improved with further refactoring, and I have some ideas for this, especially given the time constraints I didn't want to do more refactoring before getting some feedback via code review. I welcome constructive suggestions. Richard: Thanks for catching that CSS issue. I'll fix it in the next revisions. Jorg: That's tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=1558599 I started on it yesterday and will add some info to the bug today. Magnus: Thanks for the reviews and the quick turn around.
Bug 1558565 Comment 14 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
aceman: "RDF" was in pre-existing names, but I can go ahead and remove it in the refactoring patch (part 1). Thanks for the heads up on that. To respond to your other comments, rather than just copy the code and modify it, I'm following [DRY principles](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) to avoid code duplication and maintenance overhead. As you can see in the patch there is a lot of shared code between the two cases but also significant differences in the menus that need different handling. For example, <menupopup>s are children of a <menu> leading to a deeply nested structure but <panelview>s are siblings, a flat structure. You can append menu items directly to a <menupopup> but a <panelview> has a <vbox> child where menu items need to be appended. Etc. (Search for "this.localName ==" (menupopup or panelview) to see where there's branching and what the differences are.) While the code likely can be improved with further refactoring, and I have some ideas for this, especially given the time constraints I didn't want to do more refactoring before getting some feedback via code review. I welcome constructive suggestions. Richard: Thanks for catching that CSS issue. I'll fix it in the next revisions. Jorg: That's tracked here: https://bugzilla.mozilla.org/show_bug.cgi?id=1558599 I started on it yesterday and will add some info to the bug today. Magnus: Thanks for the reviews and the quick turn around.