Closed Bug 702448 Opened 13 years ago Closed 13 years ago

Create some view source UI mochitests

Categories

(Toolkit :: View Source, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla11
Tracking Status
firefox10 --- fixed

People

(Reporter: darktrojan, Assigned: darktrojan)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

I'm working on a few regression tests for the view source UI, since there are none, and it got broken a lot with the parser change.
Blocks: 699356, 469434, 700260
Blocks: 700042
Blocks: 455888
Attached patch tests (obsolete) (deleted) — Splinter Review
This patch has tests for the window title (bug 699356, currently broken so the test is a known-fail), line wrap, syntax highlighting, and tab size prefs. (I seem to be picking on you for reviews today Gavin, sorry!)
Attachment #574687 - Flags: review?(gavin.sharp)
Attachment #574687 - Flags: review?(gavin.sharp) → review?(jwein)
Comment on attachment 574687 [details] [diff] [review] tests Review of attachment 574687 [details] [diff] [review]: ----------------------------------------------------------------- Overall looks fine. I've pushed to try server: https://tbpl.mozilla.org/?tree=Try&rev=99d57a5d87e7 ::: toolkit/components/viewsource/test/browser/browser_viewsourceprefs.js @@ +13,5 @@ > + mWindow = aWindow; > + wrapMenuItem = aWindow.document.getElementById('menu_wrapLongLines'); > + syntaxMenuItem = aWindow.document.getElementById('menu_highlightSyntax'); > + > + isnot(wrapMenuItem.getAttribute("checked"), "true", "Wrap menu item not checked by default"); Is it possible to use |is(wrapMenuItem.hasAttribute("checked"), false, "Wrap menu item not checked by default");| here or do we set the |.checked| attribute to |"false"|, and if so, can we explicitly check for |"false"| instead of just checking for anything but |"true"|? According to the |simulateClick| function, we are removing the attribute, so we should just be able to use |hasAttribute|. @@ +25,5 @@ > + > +// Check that the Wrap Long Lines menu item works. > +function test1() { > + simulateClick(wrapMenuItem); > + executeSoon(function() { Is it possible to use an event listener for the |"click"| event here so we don't need the executeSoon? @@ +35,5 @@ > +} > + > +function test2() { > + simulateClick(wrapMenuItem); > + executeSoon(function() { Same question here about the possible use of a |"click"| event listener. @@ +46,5 @@ > + > +// Check that the Syntax Highlighting menu item works. > +function test3() { > + mWindow.gBrowser.addEventListener("pageshow", function() { > + mWindow.gBrowser.removeEventListener("pageshow", arguments.callee, false); Can you provide a name to the callee function so we don't have to depend on arguments.callee? According to this page, arguments.callee is deprecated in ES5 strict mode: https://developer.mozilla.org/en/JavaScript/Reference/Functions_and_function_scope/arguments/callee @@ +47,5 @@ > +// Check that the Syntax Highlighting menu item works. > +function test3() { > + mWindow.gBrowser.addEventListener("pageshow", function() { > + mWindow.gBrowser.removeEventListener("pageshow", arguments.callee, false); > + isnot(syntaxMenuItem.getAttribute("checked"), "true", "Syntax menu item unchecked"); Can we check for |hasAttribute| here as well? @@ +59,5 @@ > +} > + > +function test4() { > + mWindow.gBrowser.addEventListener("pageshow", function() { > + mWindow.gBrowser.removeEventListener("pageshow", arguments.callee, false); Same question about arguments.callee here. @@ +80,5 @@ > + ["view_source.wrap_long_lines", true], > + ["view_source.syntax_highlight", false] > + ]}, function() { > + openViewSourceWindow(source, function(aWindow) { > + isnot(wrapMenuItem.getAttribute("checked"), "true", "Wrap menu item unchecked"); Same question about |hasAttribute| here. ::: toolkit/components/viewsource/test/browser/head.js @@ +6,5 @@ > + let viewSourceWindow = openDialog("chrome://global/content/viewSource.xul", null, null, aURI); > + let gBrowser; > + waitForFocus(function() { > + gBrowser = viewSourceWindow.gBrowser; > + executeSoon(function() { Is it possible to use the |"load"| event listener all the time and remove the need for executeSoon here?
Comment on attachment 574687 [details] [diff] [review] tests Review of attachment 574687 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/viewsource/test/browser/head.js @@ +4,5 @@ > + > +function openViewSourceWindow(aURI, aCallback) { > + let viewSourceWindow = openDialog("chrome://global/content/viewSource.xul", null, null, aURI); > + let gBrowser; > + waitForFocus(function() { Could you add a "load" event listener to viewSourceWindow and when the event fires just check the target to know if the gBrowser.contentWindow has loaded instead of doing the waitForFocus, etc?
Blocks: 703965
Attached patch tests v2 (deleted) — Splinter Review
I've managed to clean up most of the stuff I put in as an attempt to just make the damn thing work. The checked attribute is sometimes false for a variety of reasons (one of which I can only explain as "a Mac thing"). I've checked for it and removed it, then used hasAttribute.
Attachment #574687 - Attachment is obsolete: true
Attachment #574687 - Flags: review?(jwein)
Attachment #575773 - Flags: review?(jwein)
Comment on attachment 575773 [details] [diff] [review] tests v2 Looks good! Thanks for adding more tests :)
Attachment #575773 - Flags: review?(jwein) → review+
Flags: in-testsuite-
Flags: in-litmus-
Target Milestone: --- → mozilla11
Comment on attachment 575773 [details] [diff] [review] tests v2 I think we'll want this on Aurora, so that the regressions from bug 482921 can land tests there. I've put it through Try on top of Aurora and there were no problems.
Attachment #575773 - Flags: approval-mozilla-aurora?
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I have a fix for bug 699356 and can see visually that the window title updates correctly. However, when openViewSourceWindow() calls the callback, aWindow.document.documentElement.getAttribute("title") returns "Nightly". Did the the window title test pass with the old View Source?
No, the attribute doesn't get updated. Only the first of those tests will pass by fixing bug 699356. I may file and fix the other bug if I can work out how. I should've mentioned it, sorry.
Comment on attachment 575773 [details] [diff] [review] tests v2 [triage comment] Approved for tests.
Attachment #575773 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
toolkit/components/viewsource/test/browser/Makefile.in was created in the tree by this bug, but was not listed in toolkit/toolkit-makefiles.sh, so I've included it here: https://hg.mozilla.org/integration/mozilla-inbound/rev/17e8e969fa09
(In reply to Ed Morley [:edmorley] from comment #14) > toolkit/components/viewsource/test/browser/Makefile.in was created in the > tree by this bug, but was not listed in toolkit/toolkit-makefiles.sh, so > I've included it here: > https://hg.mozilla.org/integration/mozilla-inbound/rev/17e8e969fa09 Seems like we would want this change also pushed to Aurora, right?
That shouldn't be necessary, since it only helps to speed up makefile creation & won't (at least in this case) cause anything to break :-)
Marking qa- as this is not something QA needs to verify. Please set to qa+ if this is not the case.
Whiteboard: [qa-]
Depends on: 728628
Blocks: 728655
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: