442972 - improve support for DOM manipulation handling in native menu system, add tests

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

Attachment: fix v1.0

This patch fixes a whole bunch of bugs related to the native menu system's response to DOM manipulations. Cumulative content append notifications work correctly with this patch, as does inserting and removing menus from top-level menu bars. Also fixes multiple bugs in our native menu testing framework, adds a DOM and native state syncing mechanism, and expands tests to include coverage for basic DOM manipulations. Also includes some code cleanup and minor optimizations.

Comment 1

[Nick Kreeger]

Comment on attachment 327625 [details] [diff] [review]
fix v1.0

r=me

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Use PRBool instead of "bool".

+    nsMenuX* newMenu = new nsMenuX();
+    if (newMenu) {
+      nsresult rv = newMenu->Create(this, this, aChild);
+      if (NS_SUCCEEDED(rv))
+        InsertMenuAtIndex(newMenu, aIndexInContainer);
+      else
+        delete newMenu;

Use nsAutoPtr<nsMenuX> and .forget() when necessary to avoid the manual delete.

Would it be a good idea to allow ForceNativeMenuReload to selectively reconstruct particular submenus or whatever actually happens when the user clicks on a menu title in the menubar? Always doing a full reconstruction might mask some bugs.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 327625 [details] [diff] [review]
fix v1.0

We discussed this on IRC and I suggested we make forceNativeMenuReload more flexible so we can more effectively test the lazy menu reconstruction, before we fix all the bugs.

Comment 4

[Josh Aas]

Attachment: fix v1.1

updated to current trunk, no changes

Comment 5

[Josh Aas]

landed on trunk, tests with lazy updating taken into account are bug 446352

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I backed this out to fix bug 447042. See changeset ced2734e6a61.

Comment 7

[Josh Aas]

Attachment: fix v1.2

Fixes the crash in DOM inspector, there was a mismatch concerning assumptions about array indexes. Not ready to go yet though, still looking into some things.

Comment 8

[Josh Aas]

Attachment: fix v1.3

Fixes another bug my originally checked-in patch introduced, a bug that prevented a sheet's parent window's menu bar from painting when a sheet closed.

Comment 9

[Nick Kreeger]

Comment on attachment 331350 [details] [diff] [review]
fix v1.3

r=me

Comment 10

[Josh Aas]

Attachment: fix v1.4

Contains additional tests for bug 447042.

Comment 11

[Josh Aas]

landed on trunk