Closed Bug 484188 Opened 11 years ago Closed 11 years ago

Port browser-chrome mochitests from browser/base/content to SeaMonkey

Categories

(SeaMonkey :: UI Design, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0b1

People

(Reporter: kairo, Assigned: kairo)

References

Details

Attachments

(1 file, 3 obsolete files)

In bug 484187 I'm porting the plain mochitests from browser/base/content, in here I'll be doing the browser-chrome ones that fit.
Attached patch port over the browser mochitests (obsolete) — Splinter Review
...aaaand here's the browser tests. Took me quite some work to get all that into shape, unfortunately, but now all tests should be working fine even on SeaMonkey.

I needed two small adjustments to how the page info dialog is being called so that the test for the feeds panel actually works, and there are a few TODOs that we may want to look into in followups.
Attachment #368350 - Flags: review?(neil)
Browser Chrome Test Summary
        Pass: 90 
        Fail: 0
        Todo: 5

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_feed_tab.js | Number of feeds listed: 2, should be 3

Here we don't detect a feed that Firefox does actually detect.

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_page_style_menu.js | link 17 with rel="alternate stylesheet" and media="allscreen" should not show up in page style menu
TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_page_style_menu.js | link 18 with rel="alternate stylesheet" and media="_all" should not show up in page style menu

Those are todo() on the Firefox side as well.

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_pluginnotification.js | Should have opened the correct window
TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_pluginnotification.js | Should have displayed the plugins pane

We don't open the Add-On Manager with the plugins pane in the case that is tested here
Comment on attachment 368350 [details] [diff] [review]
port over the browser mochitests

Looks like you forgot to hg add some of these test files (3.xml for example)

For the record we miss that feed test [compared to FF] due to not actually having a content sniffer yet for feeds (asrail is working on that iirc).
Attachment #368350 - Flags: review-
(In reply to comment #3)
> (From update of attachment 368350 [details] [diff] [review])
> Looks like you forgot to hg add some of these test files (3.xml for example)

No, those are not needed (never actually requested), they are just dummy links. if I would have forgotten any files, hg status would have choked, and it didn't.

> For the record we miss that feed test [compared to FF] due to not actually
> having a content sniffer yet for feeds (asrail is working on that iirc).

No, what we fail is that we don't detect |rel="feed"| as a feed in page info (and probably anywhere else, but page info is test here).
(In reply to comment #2)
> We don't open the Add-On Manager with the plugins pane in the case that is
> tested here
That would be bug 465771 I guess.
(In reply to comment #4)
> No, what we fail is that we don't detect |rel="feed"| as a feed in page info
> (and probably anywhere else, but page info is test here).
We detect three feeds in the browser (link toolbar uses its own code but still agrees with bookmarks menu and feed icon).
Depends on: 465771
(In reply to comment #5)
> (In reply to comment #2)
> > We don't open the Add-On Manager with the plugins pane in the case that is
> > tested here
> That would be bug 465771 I guess.

Ah, thanks, added a dependency (not a real blocker as I todo()ed that part anyway, but a good reference to have when we fix the other bug).

(In reply to comment #6)
> (In reply to comment #4)
> > No, what we fail is that we don't detect |rel="feed"| as a feed in page info
> > (and probably anywhere else, but page info is test here).
> We detect three feeds in the browser (link toolbar uses its own code but still
> agrees with bookmarks menu and feed icon).

I wonder if we should port what bug 396203 introduced and keep one single list for all those places, actually.
In any case, if browser detects three and page info only two, that's another real bug we spotted with this test porting :)
Attachment #368350 - Flags: review?(neil) → review+
Depends on: 484371
Comment on attachment 368350 [details] [diff] [review]
port over the browser mochitests

Oh wait, we can't have conditional tests here. The dependent bugs had better turn the tree orange if they don't update the test.
Attachment #368350 - Flags: review+ → review-
Attached patch updated for feed panel fix (obsolete) — Splinter Review
Updated for the feed panel fix.

Now I'm at the following:

Browser Chrome Test Summary
        Pass: 91
        Fail: 0
        Todo: 5

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_bug462289.js | tab key to tab activeElement

Yes, this sucks. Badly. See the comment in the code and my bad hackaround there. Somehow it tells me it ends up on a input element rather than the tab and I have no clue why. It worked earlier on my machine, then stopped working and I can't see what changed or what would be up in the window. I'd really like to see if this hits the TODO() on the official test machines as well.

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_page_style_menu.js | link 17 with rel="alternate stylesheet" and media="allscreen" should not show up in page style menu
TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_page_style_menu.js | link 18 with rel="alternate stylesheet" and media="_all" should not show up in page style menu

As said before, also todo() on Firefox.

TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_pluginnotification.js | Should have opened the correct window
TEST-KNOWN-FAIL | chrome://mochikit/content/browser/suite/browser/test/browser_pluginnotification.js | Should have displayed the plugins pane

bug 465771, as said before.
Attachment #368350 - Attachment is obsolete: true
Attachment #368706 - Flags: review?(neil)
Sorry for the fast followup but the tab focus thing in the last version was just too ugly to go in that way. I verified that I (still) seem to be at the html:input element inside the urlbar after issuing that VK_TAB so I added a workaround specifically for that so that we end up still testing what we actually want to test here, and that is that the tab can be focused with the tab key.
Attachment #368706 - Attachment is obsolete: true
Attachment #368712 - Flags: review?(neil)
Attachment #368706 - Flags: review?(neil)
Comment on attachment 368712 [details] [diff] [review]
v 1.2: feed panel fix and somewhat sane tab focus workaround

>+  document.getElementById("urlbar").focus();
Can you try document.getElementById("urlbar").inputField.focus(); ?
yes, .inputField.focus() works fine, thanks!
Attachment #368712 - Attachment is obsolete: true
Attachment #368719 - Flags: review?(neil)
Attachment #368712 - Flags: review?(neil)
Attachment #368719 - Flags: review?(neil) → review+
Pushed as http://hg.mozilla.org/comm-central/rev/671042296bda
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b1
Blocks: 488025
Depends on: 491624
Blocks: 491624
No longer depends on: 491624
No longer blocks: 561053
You need to log in before you can comment on or make changes to this bug.