Closed
Bug 987640
Opened 10 years ago
Closed 10 years ago
Add automated test for the functionality of the Character encoding panel in Australis
Categories
(Firefox :: Toolbars and Customization, defect)
Firefox
Toolbars and Customization
Tracking
()
RESOLVED
FIXED
Firefox 31
People
(Reporter: mihaelav, Assigned: mihaelav)
References
Details
(Whiteboard: [Australis:P-])
Attachments
(1 file, 2 obsolete files)
3.98 KB,
patch
|
mihaelav
:
review+
|
Details | Diff | Splinter Review |
This is a follow-up bug for the Character encoding button functionality in Australis - bug #967000 comment #19: "check that clicking items in the view reloads the page with a different charset, and after that has finished, the correct charset should be checked."
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Comment 2•10 years ago
|
||
Eh? This isn't a dupe.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Updated•10 years ago
|
Whiteboard: [Australis:P-]
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mihaela.velimiroviciu
Assignee | ||
Comment 3•10 years ago
|
||
Gijs, does this cover what you wanted?
Attachment #8408923 -
Flags: review?(gijskruitbosch+bugs)
Comment 4•10 years ago
|
||
Comment on attachment 8408923 [details] [diff] [review] test Review of attachment 8408923 [details] [diff] [review]: ----------------------------------------------------------------- This is the right idea! f+ because of the initialLocation thing... I'm pretty sure the code doesn't work as-is. Try looking at what other tests that load supporting files into tabs do... perhaps it makes sense to just add a tab for the page you're loading, and then removing that tab again? ::: browser/components/customizableui/test/browser_987640_charEncoding.js @@ +29,5 @@ > + //change the encoding > + let encodings = characterEncodingView.querySelectorAll("toolbarbutton"); > + let newEncoding = encodings[0].hasAttribute("checked") ? encodings[1] : encodings[0]; > + newEncoding.click(); > + yield promiseTabLoadEvent(gBrowser.selectedTab, TEST_PAGE); To avoid a race condition here (what if the load event has fired before the promise is set up?), set up the promise before newEncoding.click(), and then yield it after, so: let tabLoadPromise = promiseTabLoadEvent(gBrowser.selectedTab, TEST_PAGE); newEncoding.click(); yield tabLoadPromise; And the same for the similar code lower down, please. :-) @@ +55,5 @@ > + yield resetCustomization(); > + ok(CustomizableUI.inDefaultState, "The UI is in default state again."); > + > + // restore the initial location > + gBrowser.selectedTab.value = initialLocation; ... does this work? There is no other code in MXR that uses this, as far as I can tell...
Attachment #8408923 -
Flags: review?(gijskruitbosch+bugs) → feedback+
Assignee | ||
Comment 5•10 years ago
|
||
Updated based on review
Attachment #8408923 -
Attachment is obsolete: true
Attachment #8410321 -
Flags: review?(gijskruitbosch+bugs)
Comment 6•10 years ago
|
||
Comment on attachment 8410321 [details] [diff] [review] updated patch Review of attachment 8410321 [details] [diff] [review]: ----------------------------------------------------------------- This looks good to me. Can you push this to try to verify it works everywhere and we haven't missed anything? :-) ::: browser/components/customizableui/test/browser_987640_charEncoding.js @@ +6,5 @@ > + > +const TEST_PAGE = "http://mochi.test:8888/browser/browser/components/customizableui/test/support/test_967000_charEncoding_page.html"; > +let newTab = null; > + > +let initialLocation = gBrowser.currentURI.spec; This variable is now dead, please remove it. :-)
Attachment #8410321 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 7•10 years ago
|
||
Removed the initialLocation variable. The try tree is now closed, but I made a push yesterday with the previous version of the patch: remote: https://tbpl.mozilla.org/?tree=Try&rev=5e06395a3bc1 Carrying over the r+ from Gijs.
Attachment #8410321 -
Attachment is obsolete: true
Attachment #8410862 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7aaf686b18a8
Keywords: checkin-needed
Whiteboard: [Australis:P-] → [Australis:P-][fixed-in-fx-team]
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7aaf686b18a8
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [Australis:P-][fixed-in-fx-team] → [Australis:P-]
Target Milestone: --- → Firefox 31
You need to log in
before you can comment on or make changes to this bug.
Description
•