[mozmill] testRestoreHomeToDefault fails to validate browserHomePage URL

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: ashughes, Assigned: whimboo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozmill-test-failure], URL)

Attachments

(3 attachments)

(Reporter)

Description

8 years ago
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
(Reporter)

Updated

8 years ago
Whiteboard: [mozmill-test-failure]
(Reporter)

Comment 1

8 years ago
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
(Reporter)

Comment 2

8 years ago
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/
(Reporter)

Comment 3

8 years ago
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

8 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?
(Reporter)

Comment 5

8 years ago
> 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

8 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.
(Reporter)

Comment 7

8 years ago
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

8 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.
(Reporter)

Comment 9

8 years ago
Created attachment 452137 [details]
All-in-one Test Case

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

8 years ago
Created attachment 452142 [details]
Screenshot: Test run comparison

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

8 years ago
Created attachment 452150 [details]
Screenshot: Google

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

8 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

8 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)

Updated

8 years ago
Depends on: 572994
(Assignee)

Comment 14

8 years ago
See bug 572994 for a solution which reliable works for me.
(Reporter)

Comment 15

8 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

8 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

8 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
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
(Assignee)

Comment 18

8 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

7 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
You need to log in before you can comment on or make changes to this bug.