Closed Bug 608446 Opened 14 years ago Closed 14 years ago

Test failure in testSetToCurrentPage.js and testRestoreHomePageToDefault.js

Categories

(Mozilla QA Graveyard :: Mozmill Tests, defect)

All
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: davehunt, Assigned: davehunt)

References

Details

(Whiteboard: [mozmill-test-failure])

Attachments

(2 files, 1 obsolete file)

Browser home page preference is failing to be set when clicking on the 'Use Current Page' button for the Home Page in the General Preferences pane. Test output is: ERROR | Test Failure: {"exception": {"message": "could not validate element ID: urlbar with value http://localhost:43336/layout/mozilla.html", "lineNumber": 876, "stack": "([object Object],\"http://localhost:43336/layout/mozilla.html\")@resource://mozmill/modules/controller.js:876\n()@resource://mozmill/modules/frame.js -> file:///Users/dave/workspace/my-mozmill-tests/firefox/testPreferences/testSetToCurrentPage.js:92\n((function () {controller.open(LOCAL_TEST_PAGES[1]);controller.waitForPageLoad();controller.click(new elementslib.ID(controller.window.document, \"home-button\"));controller.waitForPageLoad();controller.sleep(30000);controller.assertValue(locationBar.urlbar, LOCAL_TEST_PAGES[0]);}))@resource://mozmill/modules/frame.js:530\n([object Object])@resource://mozmill/modules/frame.js:598\n([object Object])@resource://mozmill/modules/frame.js:641\n(\"/Users/dave/workspace/my-mozmill-tests/firefox/testPreferences/testSetToCurrentPage.js\")@resource://mozmill/modules/frame.js:480\n(\"/Users/dave/workspace/my-mozmill-tests/firefox/testPreferences/testSetToCurrentPage.js\")@resource://mozmill/modules/frame.js:653\n((function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:164\n(\"2be4607e-e3ba-11df-90bc-d830626155ec\",(function (filename, invokedFromIDE) {var runner = new Runner(new Collector, invokedFromIDE);runner.runTestFile(filename);runner.end();return true;}),[object Proxy])@resource://jsbridge/modules/server.js:168\n@:0\n@resource://jsbridge/modules/server.js:249\n", "fileName": "resource://mozmill/modules/controller.js"}} TEST-UNEXPECTED-FAIL | /Users/dave/workspace/my-mozmill-tests/firefox/testPreferences/testSetToCurrentPage.js | testHomeButton When running with debug the following appeared in the console: Warning: reference to undefined property window.documentLoaded Source File: resource://mozmill/modules/controller.js Line: 310
In case of qa-horus I was able to fix this failure by replacing the following line: http://hg.mozilla.org/qa/mozmill-tests/file/default/shared-modules/prefs.js#l138 with: > this._controller.keypress(null, 'W', {accelKey: true}); Dave, can you please check if that works for you? Not sure yet about the mozmill failure itself. Could have been raised already earlier?
I can confirm that this has fixed the failure. I can also confirm that in Firefox 3.5 and 3.6 the user can hit the ESCAPE key to dismiss the preferences pane and the modified home page will persist. In Firefox 4 Beta 6 and Minefield, hitting the ESCAPE key loses the modification.
So the issue with ESC on 4.0 with any value inside the textbox? This sounds like a regression in Firefox we should investigate. Please check former beta builds or even alpha builds + nightly builds to get a regression range.
Oh, and how does accelKey+W work on Linux?
Discovered this to be a regression, as detailed in bug 608878
The same failure should happen for the restore homepage to default test. Can you please check this and update the summary?
Hardware: x86 → All
Whiteboard: [mozmill-test-failure]
(In reply to comment #6) > The same failure should happen for the restore homepage to default test. Can > you please check this and update the summary? Absolutely correct. The test will fail if the OSX Keyboard Preferences, 'Full Keyboard Access' are set to: 'Text Box and Lists Only'
Summary: Test failure in testSetToCurrentPage.js → Test failure in testSetToCurrentPage.js and testRestoreHomePageToDefault.js
Given that the preference dialog class never knows which element has the focus, and probably shouldn't have to check this, I would say that we should change the close function to not make use of the Escape key but the AccelKey+W combo. Dave, can you please check if that works on Linux as well?
The AccelKey+W is working on Linux, however should we not ideally exercise both method of dismissing the preferences dialog in our tests? Otherwise we wouldn't pick up of this type of bug.
Somewhere there should be tests that exercise closing the dialog and checking that the values in general persist, yeah. Whether it should be -this- particular test, unsure. The tests that aren't specifically checking for on-close behavior should use the most robust close method available to decrease our maintenance.
In general tests should use a default method to close dialogs/windows or other types of widgets. As Geo mentioned I would go on here and set Accel+W as the default action. We could come up with a simple and single test which can test this specific feature.
Good point. I suggest a patch to change this test to use AccelKey+W and another patch to add tests for the persist preferences when closing the dialog. Can find some time to work on this together when I'm in MV next week?
I would prefer that we fix this bug ASAP. Means simply create a patch which fixes the close() function inside the prefs.js module. Can you do that? Before we can create any test for the escape issue, we will have to wait for a fix on bug 608878.
I will create that patch now.
Comment on attachment 488512 [details] [diff] [review] Changed the keyboard shortcut for closing preferences from Escape to Accel+W >- this._controller.keypress(null, 'VK_ESCAPE', {}); >+ this._controller.keypress(null, 'W', {accelKey: true}); Can you do a quick respin w/ lowercase 'w'? Normally I'd just edit it and land, but it'll let you try out the "obsolete patch" feature, etc. keypress() doesn't treat uppercase as shift+W, so this will work. However, we've pretty much standardized on lowercase keycodes to keep things as obvious as possible. Otherwise, looks fine.
Attachment #488512 - Flags: review?(gmealer) → review-
Attachment #488512 - Attachment is obsolete: true
Changed the keyboard shortcut for closing preferences from Escape to Accel+w
Attachment #488557 - Flags: review?(gmealer)
Comment on attachment 488557 [details] [diff] [review] Changed the keyboard shortcut for closing preferences from Escape to Accel+w Looks great, thanks for the change! I'll land it presently.
Attachment #488557 - Flags: review?(gmealer) → review+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
This is a fix for a broken behavior of the shared module and should also be fixed on the branches. The reason why it doesn't happen there is simply a focus bug in Firefox.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sorry about that--I thought the older behavior was correct and we were working around a trunk bug. I'm assigning it to myself--I'll get the patches applied to 1.9.2 and 1.9.1 as well.
Assignee: nobody → gmealer
Assignee is still Dave, who wrote the patch. I don't think any update is necessary to get this patch into the older branches.
Assignee: gmealer → dhunt
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.
Product: Testing → Mozilla QA
Version: Trunk → unspecified
Dave, can you please provide an update of the patch for the 1.9.2 branch? There is no need for 1.9.1.
(In reply to comment #24) > Dave, can you please provide an update of the patch for the 1.9.2 branch? > There is no need for 1.9.1. Patch attached for branch 1.9.2
Attachment #531511 - Flags: review?(hskupin)
Comment on attachment 531511 [details] [diff] [review] Changed the keyboard shortcut for closing preferences from Escape to Accel+w (1.9.2) v1 Thanks!
Attachment #531511 - Flags: review?(hskupin) → review+
Status: REOPENED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Product: Mozilla QA → Mozilla QA Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: