Status

()

Core
Plug-ins
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Dr.No.pt, Assigned: bsmedberg)

Tracking

({regression})

Trunk
mozilla13
x86
Windows XP
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13+ verified, firefox14+ verified)

Details

(Whiteboard: [qa+])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 607556 [details]
FlashNotLoaded.jpg

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120320 Firefox/14.0a1
Build ID: 20120320031130

Steps to reproduce:

Open http://www.meteo.pt/pt/


Actual results:

The page loaded but not flash. It says "Movie not loaded" when o right click in the flash area


Expected results:

The flash should be loaded
The referer header is missing !
Benjamin: Could this be a regression from bug 724465 ?

======================================================================
GET /bin/flash/vrs1.1/prev.meteo.sam.portugal.v2011.2.swf HTTP/1.1
Host: www.meteo.pt
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120315 Firefox/14.0a1 SeaMonkey/2.11a1
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: de-de,de;q=0.8,en-us;q=0.5,en;q=0.3
Accept-Encoding: gzip, deflate
DNT: 1
Cookie: JSESSIONID=FECB12C23CF7DE3AF8F050DA71E55047
Connection: keep-alive

Expires: Wed, 21 Mar 2012 16:37:56 GMT
Cache-Control: max-age=86400
Content-Type: application/x-shockwave-flash
Content-Length: 14379
Connection: close
HTTP/1.1 403 Forbidden
Date: Tue, 20 Mar 2012 16:28:53 GMT
Server: Apache
Content-Length: 254
Connection: close
Content-Type: text/html; charset=iso-8859-1
==================================================================================
GET /opencms/bin/flash/vrs1.1/prev.meteo.sam.portugal.v2011.2.swf HTTP/1.1
Host: www.meteo.pt
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip, deflate
Connection: keep-alive
Referer: http://www.meteo.pt/pt/
Cookie: JSESSIONID=DAFA4DCF4790D3EC9EEA4F99EB1D56F6

HTTP/1.1 302 Found
Date: Tue, 20 Mar 2012 16:51:50 GMT
Server: Apache
Location: http://www.meteo.pt/bin/flash/vrs1.1/prev.meteo.sam.portugal.v2011.2.swf
Content-Length: 256
Connection: close
Content-Type: text/html; charset=iso-8859-1
Status: UNCONFIRMED → NEW
Component: Untriaged → Plug-ins
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
QA Contact: untriaged → plugins
Version: 14 Branch → Trunk
(Assignee)

Comment 2

5 years ago
Without a regression range I don't know. It could also be bug 729009. What are the two HTTP logs you've listed?
(Assignee)

Comment 3

5 years ago
Is this problem present in Firefox 12 beta or Firefox 13 aurora?
Keywords: regressionwindow-wanted
I already started with the regression range searching after confirming.
The 2 http logs are from a nightly Seamonkey and Firefox11 as you can see by the UA string. 
I pasted the wrong http request for FF11 (the 302) but the final request shows also a referer header while the trunk GET request is without.
The request is the actual request by Gecko for the swf referenced by an object tag.

Last good nightly: 2012-01-31
First bad nightly: 2012-02-01

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f26b7bee352&tochan
ge=e18c7bc2c28e

bug 90268 ?
Keywords: regressionwindow-wanted
(Assignee)

Updated

5 years ago
Assignee: nobody → benjamin
Blocks: 90268
status-firefox13: --- → affected
status-firefox14: --- → affected
tracking-firefox13: --- → ?
tracking-firefox14: --- → ?
(Assignee)

Comment 5

5 years ago
Created attachment 607661 [details] [diff] [review]
Pass the content the embedded stream initiator, rev. 1

Josh, I'm wondering if there was a reason for passing nsnull here, or whether this was just a thinko in the original patch: this version passes tests, but my "weird side-effects" radar is going off.
Attachment #607661 - Flags: review?(joshmoz)

Comment 6

5 years ago
Comment on attachment 607661 [details] [diff] [review]
Pass the content the embedded stream initiator, rev. 1

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

I think it is a thinko, and I think I know what my original logic was (described below). Your patch seems like a fine approach, but there are two remaining issues.

You will trip the assertion at the top of nsPluginHost::NewEmbeddedPluginStream since both values can be non-null:

NS_ASSERTION(!aContent || !aInstance, "Don't pass both content and an instance to NewEmbeddedPluginStream!");

I think, and the assertion corroborates, that my original logic was that content or the instance should be passed in, never both. This was probably based on the 'if' statement at the core of nsPluginHost::NewEmbeddedPluginStreamListener. However, since the first check is for the instance then it doesn't really hurt to have an instance and content, and the content is needed for the referrer code in nsPluginHost::NewEmbeddedPluginStream. This also holds safely true when we check for an instance first in nsPluginStreamListenerPeer::InitializeEmbedded.

The second issue is that the test fails locally on Fedora 16. Oddly, it succeeds as part of a full plugin tests run but it fails when you run it individually.

I have to minus on the assertion problem and the test failure, but once that is fixed up I think we're good. Thanks for fixing this!
Attachment #607661 - Flags: review?(joshmoz) → review-

Updated

5 years ago
tracking-firefox13: ? → +
tracking-firefox14: ? → +
(Assignee)

Comment 7

5 years ago
Created attachment 610194 [details] [diff] [review]
Pass the content the embedded stream initiator, rev. 1.1

I removed the assertion. I cannot reproduce any test failures, but I've removed the unnecessary changes in test_pluginstream_referer.html (I had originally tried to combine the tests, but that seems silly). I've kicked off a try run.
Attachment #607556 - Attachment is obsolete: true
Attachment #607661 - Attachment is obsolete: true
Attachment #610194 - Flags: review?(joshmoz)

Updated

5 years ago
Attachment #610194 - Flags: review?(joshmoz) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/81975c3ca494
Target Milestone: --- → mozilla14

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/81975c3ca494
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

5 years ago
Comment on attachment 610194 [details] [diff] [review]
Pass the content the embedded stream initiator, rev. 1.1

Regression caused by (bug #): bug 90268
User impact if declined: Some websites with embedded Flash will stop working
Testing completed (on m-c, etc.): Just landed on m-c, will probably want a few days to shake out. Comes with automated tests.
Risk to taking this patch (and alternatives if risky): I believe that we are strictly returning to the previous behavior. The risk is just that the patch is wrong somehow, which given my current understanding seems pretty unlikely.
Attachment #610194 - Flags: approval-mozilla-aurora?
Comment on attachment 610194 [details] [diff] [review]
Pass the content the embedded stream initiator, rev. 1.1

[Triage Comment]
Landing for m-c/81975c3ca494 looks good, approved.
Attachment #610194 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Bsmedberg: will you be able to do this landing to aurora in the next day or can we get someone else to land on your behalf?
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/c7f981506705
status-firefox13: affected → fixed
status-firefox14: affected → fixed
Target Milestone: mozilla14 → mozilla13
Whiteboard: [qa+]
Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20100101 Firefox/13.0

Verified with Firefox 13 beta 2 on Windows XP that flash loads properly - used the STR from the Description.
status-firefox13: fixed → verified
Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20100101 Firefox/14.0

Verified with Firefox 14 beta 7 on Windows XP that flash loads properly - used the STR from the Description.
status-firefox14: fixed → verified

Updated

5 years ago
Depends on: 776451
You need to log in before you can comment on or make changes to this bug.