Closed Bug 395533 Opened 14 years ago Closed 14 years ago

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9beta1

People

(Reporter: Gavin, Assigned: bzbarsky)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

A recent-ish 1.8 branch build displays a feed preview when visiting http://youtube.com/rss/global/top_favorites.rss . A current trunk build does not. Phil thought this might have been a dupe of bug 364677, but that's a regression in 2.0.0.1 as far as I can tell, while this doesn't seem to have regressed on the branch at all, and the feed does appear to have a <link> in the <channel>.
Regression range:
http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&branchtype=match&date=explicit&mindate=2007-08-20+18%3A00&maxdate=2007-08-21+06%3A00

It's not bug 385178 (tested by local backout), so I suspect bug 389677 (the feed is served as text/plain). My local backout of that patch is giving me trouble, though, I'll confirm in a bit.
Component: RSS Discovery and Preview → Networking
Product: Firefox → Core
QA Contact: rss.preview → networking
Target Milestone: --- → mozilla1.9 M9
Assignee: nobody → bzbarsky
Summary: No feed preview for http://youtube.com/rss/global/top_favorites.rss → [FIX]No feed preview for http://youtube.com/rss/global/top_favorites.rss
Attached patch Proposed fix (obsolete) — Splinter Review
Attachment #280708 - Flags: superreview?(cbiesinger)
Attachment #280708 - Flags: review?(cbiesinger)
Duplicate of this bug: 395792
Hrm, I think it would be better to throw an error instead, as documented in nsIContentSniffer.idl.
Hmm...  Good point.
Attached patch With that changeSplinter Review
Attachment #280708 - Attachment is obsolete: true
Attachment #280948 - Flags: superreview?(cbiesinger)
Attachment #280948 - Flags: review?(cbiesinger)
Attachment #280708 - Flags: superreview?(cbiesinger)
Attachment #280708 - Flags: review?(cbiesinger)
Attachment #280948 - Flags: superreview?(cbiesinger)
Attachment #280948 - Flags: superreview+
Attachment #280948 - Flags: review?(cbiesinger)
Attachment #280948 - Flags: review+
Comment on attachment 280948 [details] [diff] [review]
With that change

Requesting 1.9 approval for simple regression fix
Attachment #280948 - Flags: approval1.9?
Attachment #280948 - Flags: approval1.9? → approval1.9+
Checked in.  I'm not sure how to test it.  Does feed preview give a different DOM somehow?  That is, could a mochitest that loads such a URI in a subframe reliably detect that a feed has been sniffed?

Or is this just unit-testable somehow?
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
documentElement.id == "feedHandler" seems like the easiest test for feed preview, offhand.
Is there an obvious DOM that I can try serving as text/plain that would get sniffed as a feed?
I was going to attach this, but the minimal feed is actually pretty minimal:

<rss version="2.0">
  <channel>
    <link>http://example.org/</link>
    <title>t</title>
  </channel>
</rss>

(It could be slightly more minimal, with <title/>, but I think that's maybe a bug.) Load it in an iframe, if the iframe's documentElement is an html with id == "feedHandler" then you pass - it displays as plain text in my build from this morning, and gets feed preview in my build from tonight.
Checked in the test.
Flags: in-testsuite? → in-testsuite+
Is it just me? This bug is giving me a failure when I run it locally. I built a copy of FF, checked out 20071202_202630_PST, with mozconfig:
mk_add_options MOZ_CO_PROJECT=browser
ac_add_options --enable-debug
ac_add_options --enable-application=browser
ac_add_options --enable-test
ac_add_options --enable-gtktest
ac_add_options --enable-mochitest
ac_add_options --enable-libIDLtest
ac_add_options --enable-glibtest

I am on Mac OS X 10.4.10 with a fairly standard tool set.

I launched with "perl ./runtests.pl --autorun".

I see this in the web page when I click on the test:

not ok - Text didn't get sniffed as a feed? got "", expected "feedHandler"

and this in the stdout:

--WEBSHELL 0x157e5e80 == 6
++DOMWINDOW == 40
++WEBSHELL 0x2f750b60 == 7
++DOMWINDOW == 41
[loaded plugin /Library/Internet Plug-Ins/QuickTime Plugin.plugin]
++DOMWINDOW == 42
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
###!!! ASSERTION: cannot call on a dirty frame not currently being reflowed: '!NS_SUBTREE_DIRTY(this) || (GetStateBits() & NS_FRAME_IN_REFLOW)', file nsFrame.cpp, line 556
--DOMWINDOW == 41
--DOMWINDOW == 40
--DOMWINDOW == 39
--DOMWINDOW == 38
--DOMWINDOW == 37
--DOMWINDOW == 36

Only one other test is failing. In case it is related, it is:
/tests/content/html/document/test/test_bug391777.html
The test was also failing for me a couple weeks back, Fedora 8.  I did a little debugging but didn't get far enough to figure out the problem, and I haven't had time to investigate since then.
Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007122704 Minefield/3.0b3pre.
Status: RESOLVED → VERIFIED
For some reason the test fails on my 32 bit Fedora 7, but not on 64 bit Fedora 4 machine.
er, 64 bit Fedora 7
Do you see the failure on the site too?  Or just the test?  Does the test obviously fail if you load it standalone?  Want to give debugging it a stab?
Note that we just undid this patch by restricting the types feed sniffing will sniff.
You need to log in before you can comment on or make changes to this bug.