Closed
Bug 1493483
Opened 6 years ago
Closed 6 years ago
In production code, use nsIBrowserSearchService::curentEngine instead of defaultEngine
Categories
(Firefox :: Search, defect, P3)
Firefox
Search
Tracking
()
RESOLVED
FIXED
Firefox 65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: Siddhant085, Assigned: Siddhant085, Mentored)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Safari/605.1.15
Assignee | ||
Updated•6 years ago
|
Component: Untriaged → Search
Priority: -- → P3
Version: unspecified → Trunk
Assignee | ||
Updated•6 years ago
|
Mentor: standard8
Updated•6 years ago
|
Assignee: nobody → dpsrkp.sid
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: [meta] (not test cases) Update the assignment and use of nsIBrowserSearchService::curentEngine (in favour of defaultEngine) → In production code, use of nsIBrowserSearchService::curentEngine instead of defaultEngine
Assignee | ||
Comment 1•6 years ago
|
||
Was defaultEngine and currentEngine different in the past. I found this code
// If the search bar is visible, use the current engine, otherwise, fall
// back to the default engine.
if (isElementVisible(this.searchBar))
engine = Services.search.defaultEngine;
else
engine = Services.search.defaultEngine;
Flags: needinfo?(standard8)
Comment 2•6 years ago
|
||
Yes, they were different at some stage. So we should clean up that code and drop the if statement.
Flags: needinfo?(standard8)
Updated•6 years ago
|
Summary: In production code, use of nsIBrowserSearchService::curentEngine instead of defaultEngine → In production code, use nsIBrowserSearchService::curentEngine instead of defaultEngine
Assignee | ||
Comment 3•6 years ago
|
||
nsIBrowserSearchService::currentEngine and nsIBrowserSearchService::defaultEngine are the same thing.
The use of defaultEngine makes more sense and thus we are phasing out the use of currentEngine and replace it with defaultEngine.
Updated•6 years ago
|
Attachment #9015495 -
Attachment description: Use nsIBrowserSearchService::curentEngine instead of defaultEngine (in production code) → Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c35bab726b03
Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8
Comment 5•6 years ago
|
||
Backed out for multiple browser-chrome failures e.g browser_extension_controlled.js
backout: https://hg.mozilla.org/integration/autoland/rev/dd418f0ef29cf4082b1379211b601fcf950836da
push with failures https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed,busted,exception,running,pending,runnable&selectedJob=204306963&revision=c35bab726b030312c53f48a654d5fcc2e1264ac2&group_state=expanded
failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=204306963&repo=autoland&lineNumber=2425
[task 2018-10-09T19:44:23.235Z] 19:44:23 INFO - Console message: [JavaScript Error: "TypeError: Services.search.deafultEngine is undefined; can't access its "name" property" {file: "chrome://browser/content/parent/ext-chrome-settings-overrides.js" line: 90}]
[task 2018-10-09T19:44:23.235Z] 19:44:23 INFO - Buffered messages finished
[task 2018-10-09T19:44:23.235Z] 19:44:23 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Test timed out -
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Extension not fully unloaded at test shutdown -
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - Stack trace:
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:112
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - chrome://mochikit/content/browser-test.js:nextTest:695
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1190
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - setTimeout handler*chrome://mochikit/content/browser-test.js:Tester_execTest:1152
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:986
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-10-09T19:44:23.236Z] 19:44:23 INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Extension left running at test shutdown -
Flags: needinfo?(dpsrkp.sid)
Assignee | ||
Comment 6•6 years ago
|
||
One problem causing browser_extension_controlled.js to fail is the incorrect spelling of default in ext-chrome-settings-overrides.js. I have fixed that and retried the test on my local system. It is still timing out. I am looking into the problem.
Flags: needinfo?(dpsrkp.sid)
Assignee | ||
Comment 7•6 years ago
|
||
The test is timing out because of
INFO Failed to retrieve MOZ_UPLOAD_DIR env var
pushing the changes to run the tests on try server.
Comment 8•6 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2920d79f0e6be184085c35ea19eec8272deb743e
Siddhant: can you post an updated patch which fixes the bitrot in PlacesSearchAutocompleteProvider.jsm ?
Updated•6 years ago
|
Flags: needinfo?(dpsrkp.sid)
Assignee | ||
Comment 9•6 years ago
|
||
Hey Mark,
I went through the treeherder test summary and I see that uriloader/exthandler/tests/mochitest/browser_auto_close_window.js is failing and it also has a bug filed for it (https://bugzilla.mozilla.org/show_bug.cgi?id=1472761). I can't see any errors involving PlacesSearchAutocompleteProvider.jsm.
Flags: needinfo?(dpsrkp.sid) → needinfo?(standard8)
Comment 10•6 years ago
|
||
(In reply to Siddhant [:Siddhant085] from comment #9)
> I went through the treeherder test summary and I see that
> uriloader/exthandler/tests/mochitest/browser_auto_close_window.js is failing
> and it also has a bug filed for it
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1472761).
I believe that failure was actually a side-effect of something that was wrong on the tree at that time.
> I can't see any
> errors involving PlacesSearchAutocompleteProvider.jsm.
Did you pull the latest and rebase it? If so, if you could push again from the latest just to make sure that would be useful.
I'll push it to try one more time just to make sure there isn't an issue with that auto close test.
Flags: needinfo?(standard8)
Comment 11•6 years ago
|
||
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/408f7a1a2d0f
Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8
Comment 12•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment 13•6 years ago
|
||
Commits pushed to master at https://github.com/mozilla/activity-stream
https://github.com/mozilla/activity-stream/commit/4b365ad515f7379fbc4bb370f976418009aa518f
Backport Bug 1493483 - Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8
https://github.com/mozilla/activity-stream/commit/73f173931bd55c1e122454be563ed53b769041ff
Bug 1493483 - Fix unit tests to stub .defaultEngine
You need to log in
before you can comment on or make changes to this bug.
Description
•