Closed
Bug 1270078
Opened 8 years ago
Closed 8 years ago
AutocompleteResults.get_matching_text() broken due to the new awesomebar design
Categories
(Testing :: Firefox UI Tests, defect)
Tracking
(firefox48 fixed, firefox49 fixed)
RESOLVED
FIXED
mozilla49
People
(Reporter: whimboo, Assigned: whimboo)
References
Details
(Keywords: regression)
Attachments
(1 file)
Via bug 1181078 we got a new design for the awesomebar. As result some of our tests are failing now: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=Firefox%20UI&filter-tier=2&filter-tier=3&selectedJob=27215679 TEST-UNEXPECTED-ERROR | test_toolbars.py TestAutoCompleteResults.test_matching_text | JavascriptException: JavascriptException: TypeError: arguments[0].boxObject.firstChild.childNodes[1] is undefined TEST-UNEXPECTED-ERROR | test_suggest_bookmarks.py TestStarInAutocomplete.test_star_in_autocomplete | JavascriptException: JavascriptException: TypeError: arguments[0].boxObject.firstChild.childNodes[1] is undefined
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Summary: Auto-complete tests are broken due to new awesome design → AutocompleteResults.get_matching_text() broken due to the new awesomebar design
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/50497/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/50497/
Attachment #8748725 -
Flags: review?(mak77)
Assignee | ||
Comment 2•8 years ago
|
||
Interesting. The try build is still failing for one case: TEST-UNEXPECTED-FAIL | test_toolbars.py TestAutoCompleteResults.test_matching_text | AssertionError: u'a' not found in 'moz' I will investigate.
Assignee | ||
Comment 3•8 years ago
|
||
Given by Marco we are doing it right for waiting until the search is complete: http://mxr.mozilla.org/mozilla-central/source/testing/puppeteer/firefox/firefox_puppeteer/ui/browser/toolbars.py#384 But we still seem to have a race condition here when you quickly search for different terms: input: "about" opening autocomplete... done waiting for search complete 2 4 done all matches: [u'About', u'about'] closing... closed input: "moz" opening autocomplete... done waiting for search complete 2 4 done all matches: [u'moz'] all matches: [u'moz'] all matches: [u'moz'] all matches: [u'moz'] all matches: [u'moz'] closing... closed input: "bout" opening autocomplete... done waiting for search complete 2 4 done all matches: [u'bout', u'bout'] all matches: [u'moz'] For the last iteration the second result has the input from the last iteration emphasized. Drew, do you have an idea why that happens? Could this actually be a bug?
Flags: needinfo?(adw)
Comment 4•8 years ago
|
||
It looks like the final iteration is seeing one item left over from the previous iteration. Depending on how your test works, that's entirely possible because the popup collapses results from previous searches asynchronously, after the search has completed. It looks like your problem is here: http://mxr.mozilla.org/mozilla-central/source/testing/puppeteer/firefox/firefox_puppeteer/ui/browser/toolbars.py#358 I bet that that code is running before the items from the previous iteration are collpased. Instead of looking at uncollapsed items, you should be checking items up to index popup._matchCount, since _matchCount is updated immediately when a search finishes. I can't say why you're hitting this now and weren't before, but the fact that you weren't before was probably just lucky timing. I could be wrong about all this since I haven't tried to verify it, but that's what it looks like to me.
Flags: needinfo?(adw)
Assignee | ||
Comment 5•8 years ago
|
||
That was a great hint, Drew! That definitely fixed the problem! :) A new patch is coming up soon.
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8748725 [details] MozReview Request: Bug 1270078 - Fix AutocompleteResults.get_matching_text() for new awesomebar design. r?adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50497/diff/1-2/
Attachment #8748725 -
Attachment description: MozReview Request: Bug 1270078 - Fix AutocompleteResults.get_matching_text() for new awesomebar design. r?mak → MozReview Request: Bug 1270078 - Fix AutocompleteResults.get_matching_text() for new awesomebar design. r?adw
Attachment #8748725 -
Flags: review?(mak77) → review?(adw)
Updated•8 years ago
|
Attachment #8748725 -
Flags: review?(adw) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8748725 [details] MozReview Request: Bug 1270078 - Fix AutocompleteResults.get_matching_text() for new awesomebar design. r?adw https://reviewboard.mozilla.org/r/50497/#review47915 Glad that fixed it. I don't know these tests at all, so take my r+ with a grain of salt, but as far as the autocomplete parts of this go, this looks OK to me. ::: testing/puppeteer/firefox/firefox_puppeteer/ui/browser/toolbars.py:362 (Diff revision 2) > let node = arguments[0]; > - for (let i = 0; i < node.itemCount; ++i) { > + let count = arguments[1]; > + > + for (let i = 0; i < count; ++i) { > let item = node.getItemAtIndex(i); > if (!item.hasAttribute("collapsed")) { I think this should always be true, so you should be able to remove this conditional and just unconditionally push the item, if you'd like.
Assignee | ||
Comment 8•8 years ago
|
||
https://reviewboard.mozilla.org/r/50497/#review47915 > I think this should always be true, so you should be able to remove this conditional and just unconditionally push the item, if you'd like. Oh, right. Tested that and it works. I will get it removed then. Thanks for noticing that.
Assignee | ||
Comment 9•8 years ago
|
||
Comment on attachment 8748725 [details] MozReview Request: Bug 1270078 - Fix AutocompleteResults.get_matching_text() for new awesomebar design. r?adw Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50497/diff/2-3/
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/999dad371641
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee | ||
Comment 12•8 years ago
|
||
Tests are fixed on inbound and mozilla-central. Lets backport it also to mozilla-aurora. Thanks.
Whiteboard: [checkin-needed-aurora]
Comment 13•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/6772bf149f18
Whiteboard: [checkin-needed-aurora]
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•