794067 - Sync Feed Preview with Firefox as at 2012/09

Status: RESOLVED FIXED

(SeaMonkey :: Feed Discovery and Preview, defect)

Description

[Philip Chee]

To Do.

Comment 1

[Philip Chee]

Attachment: Patch v1.0 Sync changes.

Changes:

FeedConverter.js:
Bug 714631 Feed converter should check HTTP channel response status before proceeding.
Bug 783738 Use Components.results.NS_BINDING_ABORTED instead of hardcoding the value.

WebContentConverter.js:
Bug 520374 WebContentConverter.js needlessly includes debug.js. [NS_ASSERT]

nsFeedSniffer.h:
Bug 758992 Make the classes which use the XPCOM nsISupports implementation macros final, to avoid the warning about deleting using a pointer to a base class with virtual functions and no virtual dtor (browser parts)

nsFeedSniffer.cpp:
Bug 739040 Crash in nsStringBuffer::Realloc() during Feed sniffing (on second visit ?)
Bug 739040 Use the length of the decoded string, if no more than 512 bytes, to sniff for feeds.
Bug 739040 followup: properly enforce the sniffing limit on uncompressed feeds. Plus tests.

$ TEST_PATH=suite/browser/test/  pymake -C ../objdir-sm/ mochitest-plain

67 INFO TEST-START | Shutdown
68 INFO Passed: 54
69 INFO Failed: 0
70 INFO Todo:   2
71 INFO SimpleTest FINISHED
72 INFO TEST-INFO | Ran 0 Loops
73 INFO SimpleTest FINISHED

Note: excludes context menu tests in the same directory.

Comment 2

[Ian Neal]

(In reply to Philip Chee from comment #1)
> Created attachment 664488 [details] [diff] [review]
> Patch v1.0 Sync changes.
> 
> Changes:
> 
> Bug 783738 Use Components.results.NS_BINDING_ABORTED instead of hardcoding
> the value.
Should this not cover changes to nsBrowserContentHandler.js, nsBrowserStatusHandler.js and uploadProgress.js too? There is EdLinkChecker.js too but that is in shared code (with TB) and is wider than just NS_BINDING_ABORTED so warrants a separate bug.

Comment 3

[Ian Neal]

Comment on attachment 664488 [details] [diff] [review]
Patch v1.0 Sync changes.

r=me with previously mentioned NS_BINDING_ABORTED changes made.

Comment 4

[Philip Chee]

> r=me with previously mentioned NS_BINDING_ABORTED changes made.
I'll do this in a separate bug with a more informative summary.

Comment 5

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/a72e2158740a

Comment 6

[Philip Chee]

Attachment: Patch v2.0 Branch safe patch.

[Approval Request Comment]
Regression caused by: N/A
User impact if declined: Crashes in nsStringBuffer::Realloc() during Feed sniffing plus other stuff discussed over IRC.
Testing completed : comm-central, plus has tests. 
Risk to taking this patch (and alternatives if risky): probably none. Ports patches that have been baked on Firefox since Mozilla14.
String changes made by this patch: None.

Comment 7

[Justin Wood (:Callek)]

Comment on attachment 666581 [details] [diff] [review]
Patch v2.0 Branch safe patch.

Review of attachment 666581 [details] [diff] [review]:
-----------------------------------------------------------------

http://hg.mozilla.org/releases/comm-beta/rev/f4d39ec8e3cd

Comment 8

[Philip Chee]

Pushed:
http://hg.mozilla.org/releases/comm-aurora/rev/5a9c180c8930

Comment 9

[Philip Chee]

>> r=me with previously mentioned NS_BINDING_ABORTED changes made.
> I'll do this in a separate bug with a more informative summary.

I filed Bug 797410 to cover this. I also marked this a GFB.

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

I guess the below mochitest-5 failure stems from this bug. I think test_maxSniffing needs to be turned into a chrome test (like the FF original) to fix the permission problem:

WINNT 5.2 comm-central-trunk debug test mochitests-5/5 on 2012/11/07 10:04:43
s: sea-win32-04
3775 ERROR TEST-UNEXPECTED-FAIL | /tests/suite/browser/test/test_maxSniffing.html | an unexpected uncaught JS exception reported through window.onerror - Error: Permission denied to access property 'documentElement' at http://mochi.test:8888/tests/suite/browser/test/test_maxSniffing.html:27

I don't have a working build right now but will look into this within the next few days (unless someone reading this gets to it faster). In its own bug of course, depend on this one and to be pushed to everything up to/including at least beta. Once it's fixed, mochitest-5 should be green again. :-)