Closed
Bug 572786
Opened 14 years ago
Closed 14 years ago
[mozmill] testRestoreHomeToDefault fails to validate browserHomePage URL
Categories
(Mozilla QA Graveyard :: Mozmill Tests, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: u279076, Assigned: whimboo)
References
()
Details
(Whiteboard: [mozmill-test-failure])
Attachments
(3 files)
MODULE: firefox\testPreferences\testRestoreHomepageToDefault.js TEST: testRestoreHomeToDefault (107) FAIL: could not validate element ID: browserHomePage with value http://www.mozilla.org/ BRANCH: default, 1.9.2, 1.9.1 PLATFORMS: Win32-only
For whatever reason, this test fails silently on brasstacks. I can clearly see the error locally though.
Assignee: nobody → anthony.s.hughes
Status: NEW → ASSIGNED
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.6pre) Gecko/20100615 Namoroka/3.6.6pre (.NET CLR 3.5.30729) I found out the problem... There is an inconsistency between what is resides in browserconfig.properties and what is returned by PrefsAPI.getPref(): >about:config:browser.startup.homepage resource:/browserconfig.properties >browserconfig.properties:browser.startup.homepage http://www.mozilla.org/projects/namoroka/ >PrefsAPI.preferences.getPref("browser.startup.homepage", "") http://www.mozilla.org/
In addition, this test only checks the pref is reverted, not that the default homepage loads after reverting this pref. We should add this check once the above issue is resolved.
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #2) > >browserconfig.properties:browser.startup.homepage > http://www.mozilla.org/projects/namoroka/ > > >PrefsAPI.preferences.getPref("browser.startup.homepage", "") > http://www.mozilla.org/ So what does getProperty("resource:/browserconfig.properties", "browser.startup.homepage") return?
> So what does getProperty("resource:/browserconfig.properties",
> "browser.startup.homepage") return?
That gets us the full URL. At least that gives me a starting point to work from. I'll do some more investigation and follow up.
Assignee | ||
Comment 6•14 years ago
|
||
The failure is probable in the second line here: var browserHomepage = new elementslib.ID(controller.window.document, "browserHomePage"); controller.assertValue(browserHomepage, UtilsAPI.getDefaultHomepage()); Instead of calling assertValue we should use waitForEval. I assume there is a small delay after clicking the restore to default button.
After some investigation, I'm not sure it's a Mozmill bug. It may be that we are incorrectly using nsIPrefsBranch, but see this: var branch = Cc["@mozilla.org/preferences-service;1"] .getService(Ci.nsIPrefBranch); Cu.reportError("Pref: " + branch.getCharPref("browser.startup.homepage")); The above code returns "http://www.mozilla.org/", not the full URL. So it appears that our Mozmill code is not altering the data it gets back from nsIPrefBranch. It's the data that nsIPrefBranch is returning that's the problem.
Assignee | ||
Comment 8•14 years ago
|
||
Can you please write a little test which outputs each single step and content we retrieve with our function? Eventually that would bring us closer to the problem, ie. which properties file contains the above pref and what is the default homepage reported by the properties.file.
Here is an all-in-one test case. I've moved all the needed API methods so they are local methods to this test module. The functional code within has been unchanged. Interestingly enough, this test displays the URLs in full. This needs further investigation.
Reporter | ||
Comment 10•14 years ago
|
||
Here is a screenshot showing the debug output for two test runs. The first is using the test from the repository. The second is from the test case I have attached. The only differences, code-wise is the the repository test makes calls to API objects instead of using local functions, and the minimized test does not have all the stuff that happens before this check is performed. I'll work on adding that stuff at the beginning to see if it affects the outcome at all.
Reporter | ||
Comment 11•14 years ago
|
||
I found out where "http://www.mozilla.org/" is coming from. It's the Link object defined at the beginning of the test. var testRestoreHomeToDefault = function() { // Open a web page for the temporary home page controller.open('http://www.google.com/'); controller.waitForPageLoad(); var link = new elementslib.Link(controller.tabs.activeTab, "Google"); It looks to me like the Pref is being returned before it is reset, somehow...
Reporter | ||
Comment 12•14 years ago
|
||
Ok, I think I finally figured this out...the problem is in the logic of getDefaultHomepage(). By default we check about:config for a .properties file. If it's not found we use the set pref. Since this pref has already been set, we are really just checking against the same page. We always want to load from a .properties file. For Win/Mac, this means browserconfig.properties. For Ubuntu, this means it's Ubufox file.
Reporter | ||
Comment 13•14 years ago
|
||
So here is the situation. We need to use UtilsAPI.getProperty() to retrieve the real default home page. However, the properties file is different in 3 cases: 1. Minefield: chrome://branding/locale/browserconfig.properties 2. Namoroka/Shiretoko: resource:/browserconfig.properties 3. Ubuntu: chrome://ubufox/locale/ubufox.properties Based on this, I think we need to refactor UtilsAPI.getDefaultHomepage to always check the .properties file (checking the pref is not enough). I also think we will need a separate patch for default-branch and 1.9.1/1.9.2. In addition, need to figure out a better way to handle the Ubuntu scenario. This also has a trickle-down effect to all tests which use this method: ./testPreferences/testRestoreHomepageToDefault.js ./testSecurity/testSafeBrowsingNotificationBar.js ./testSecurity/testSafeBrowsingWarningPages.js ./testSecurity/testUntrustedConnectionErrorPage.js ./testTabbedBrowsing/testNewWindow.js Henrik, please advise. Henrik, please advise how to proceed with patches. I'm thinking we need a unique patch for default-branch.
Assignee | ||
Comment 14•14 years ago
|
||
See bug 572994 for a solution which reliable works for me.
Reporter | ||
Comment 15•14 years ago
|
||
(In reply to comment #14) > See bug 572994 for a solution which reliable works for me. Codewise, the patch in bug 572994 is fine. However, this does not change the problem I was seeing here. The problem is that we are grabbing the pref from about:config first. 1. First launch firefox => pref=browserconfig.properties 2. Set the pref to something 3. Restore to default => pref=default URL (Minefield, Namoroka, Shiretoko project pages) UtilsAPI.getDefaultHomepage() needs to load from browserconfig.properties ALWAYS and not check the pref in about:config
Assignee | ||
Comment 16•14 years ago
|
||
(In reply to comment #15) > Codewise, the patch in bug 572994 is fine. However, this does not change the > problem I was seeing here. The problem is that we are grabbing the pref from > about:config first. I hope the new version which has been checked-in a moment ago fixes the problem now.
Reporter | ||
Comment 17•14 years ago
|
||
The fix for bug 572994 appears to have fixed this bug. I cannot reproduce this failure locally anymore. Nor do I see it in the recent brasstacks reports. Resolving WORKSFORME since no patch was applied to this bug directly.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → WORKSFORME
Assignee | ||
Comment 18•14 years ago
|
||
Correct. My patch on bug 572994 fixed it. As long as we know which patch fixed a problem we can mark it as fixed instead of WFM.
Assignee: anthony.s.hughes → hskupin
Resolution: WORKSFORME → FIXED
Assignee | ||
Comment 19•14 years ago
|
||
Mass move of Mozmill Test related project bugs to newly created components. You can filter out those emails by using "Mozmill-Tests-to-MozillaQA" as criteria.
Component: Preferences → Mozmill Tests
Product: Firefox → Mozilla QA
QA Contact: preferences → mozmill-tests
Version: Trunk → unspecified
Updated•5 years ago
|
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•