Closed Bug 581568 Opened 9 years ago Closed 9 years ago

fix browser_bookmarks.js to work with e10s

Categories

(Firefox for Android Graveyard :: General, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: vingtetun)

Details

Attachments

(1 file, 2 obsolete files)

a bit of bitrot has crept into this test file, here is a cleanup patch.
Attachment #459933 - Flags: review?(21)
Comment on attachment 459933 [details] [diff] [review]
e10s and bitrot cleanup for browser_bookmarks.js

Good to see the messageManager used for pageshow :)
Attachment #459933 - Flags: review?(21) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/21d4ced24389
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
note, these are broken again, due to bug 581252
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: FIXED → ---
Attached patch Patch (obsolete) — Splinter Review
My bad, I should have seen that during my review. To be forgiven here a patch to fix the breakage.
Attachment #460475 - Flags: review?(mark.finkle)
Comment on attachment 460475 [details] [diff] [review]
Patch

>diff -r 21d4ced24389 chrome/tests/browser_bookmarks.js
>     // Need to wait until the page is loaded
>     messageManager.addMessageListener("pageshow",
>-    function() {
>-      if (gCurrentTest._currenttab.browser.currentURI.spec != "about:blank") {
>-        messageManager.removeMessageListener("pageshow", arguments.callee);
>+    function(aMessage) {
>+      if (gCurrentTest._currentTab.browser.currentURI.spec != "about:blank") {
>+        messageManager.removeMessageListener(aMessage.name, arguments.callee);
>         gCurrentTest.onPageReady();

Do we really need this change?

Looks good otherwise. We should file a followup bug to add testing the contextmenu too. I am OK with just getting these tests landed first.
Attachment #460475 - Flags: review?(mark.finkle) → review+
Attached patch Patch v0.2Splinter Review
This patch remove toggleManage everywhere (in such a way that browser_bookmarks.js and browser_bookmarks_star.js pass with success) and remove the unused browser_bookmarks_folder.js
Attachment #459933 - Attachment is obsolete: true
Attachment #460475 - Attachment is obsolete: true
Attachment #460883 - Flags: review?(mark.finkle)
Attachment #460883 - Flags: review?(mark.finkle) → review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/199c8b2f7618
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
bugspam
Assignee: nobody → 21
You need to log in before you can comment on or make changes to this bug.