582745 - Errors during browser_bookmarks_star.js tests

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Bookmarks, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js | Test adding tags via star icon
TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js | Console message: [JavaScript Error: "this.control is null" {file: "chrome://browser/content/bindings.xml" line: 478}]
TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js | Console message: [JavaScript Error: "document.getElementById("bookmark-item") is null" {file: "chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js" line: 147}]
TEST-INFO | chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js | Console message: [JavaScript Error: "content is null" {file: "chrome://browser/content/content.js" line: 480}]
TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/mobile/chrome/browser_bookmarks_star.js | Timed out

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This patch resolve the bookmark_star timeout and a few others problem.
The change in browser-ui is to hide the popup when the url change (and I need to clean that part) otherwise some test are failing. 
The patch needs probably some small change before a real review

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 466662 [details] [diff] [review]
Patch

Approach looks good. Lots of test cleanup too

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This patch change stuff into different places:
 * Add a message listener onto the bookmark popup to hide it if the page change
 * rewrite tests to use pageshow and avoid the waitFor failure
 * replace some tests as "todo" because there are failing when use in directly from the testsuite but not when launch individually (one of them was also blocking the testsuite, a problem with synthesize event and e10s I assume)

Now, testsuite is finally green! 
hmm, well I admit to have seen a random failure into the browser_viewport.js test and one into browser_navigation.js but this is hard to reproduce and this can be filed into different bug (this is not from my modifications)

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 467022 [details] [diff] [review]
Patch

r+ but do not land until alpha1 has shipped

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/7be60f729dba