Closed
Bug 229168
Opened 21 years ago
Closed 18 years ago
Save Page (with mms:// embedded media) invokes Windows Media Player
Categories
(Core Graveyard :: Embedding: APIs, defect, P2)
Core Graveyard
Embedding: APIs
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9alpha3
People
(Reporter: isaachh, Assigned: sciguyryan)
References
(Blocks 1 open bug, )
Details
(Keywords: top100)
Attachments
(2 files, 7 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20031222 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20031222 . Reproducible: Always Steps to Reproduce: 1. Go to the URL 2. Ctrl+S and OK Actual Results: Windows Media Player (WMP) window are invoked. Expected Results: Just save the page without annoying WMP window This happens only when the saved page has embedded media with src="mms://".
Comment 1•21 years ago
|
||
I was able to save the page as expected without media player side effects. WinXP Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20031222
Comment 2•21 years ago
|
||
wfm Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7a) Gecko/20031222 When you load the page, WMP starts, doesn´t matter, if you press Ctrl+S or not. marking bug INVALID. feel free to reopen, if you can tell me how you succeeded to save the page using CTRL+S without loading (and rendering) it first. I don´t understand the steps to repeat: When I load the URL, WMP starts. When I abort WMP, Mozilla has some difficulties to load other pages, and then an alert comes up that the plugin had done something, I better should restart Mozilla. When I use TAB to select the URL: or the real URL in the URL-box, and press Ctrl-S, I get a box offering me to download show_bug.cgi.html So I put the cursor over URL:, right-clicked, selected Save Link Target As, and the file was saved without starting WMP. the code contains <PARAM NAME=AutoStart VALUE=TRUE>, so if you load the page, WMP is started automatically, before you can type your Ctrl+S to save the page. If you want to see the source code of a page, or save it, without rendering it, prefix the URL with view-source: view-source:http://www.gamespot.com/live/finder/finder_ad_mi.html <OBJECT ID="WMPlay" width=320 height=312 classid="CLSID:6BF52A52-394A-11d3-B153-00C04F79FAA6" codebase="http://activex.microsoft.com/activex/controls/mplayer/en/nsmp2inf.cab#Version=5,1,52,701" standby="Loading Microsoft Windows Media Player components..." type="application/x-oleobject"> <PARAM name="URL" value="mms://someLongPath/ad_atart_mios.asf"> <PARAM name=DisplaySize value=0> <PARAM NAME=ShowControls VALUE=1> <PARAM NAME=ShowDisplay VALUE=0> <PARAM NAME=ShowStatusBar VALUE=1> <PARAM NAME=AutoStart VALUE=TRUE> <embed width="320" height="312" type="application/x-mplayer2" pluginspage="http://www.microsoft.com/Windows/Downloads/Contents/Products/MediaPlayer/" src="mms://someLongPath/ad_atart_mios.asf" Name="GameSpot Live" ShowControls="1" AutoStart=True ShowDisplay="0" ShowStatusBar="1"> </embed> </object>
Status: UNCONFIRMED → RESOLVED
Closed: 21 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 3•21 years ago
|
||
Reopening by reporter. It's weird that I can consistently reproduce this bug with a clean profile in trunk build, mozilla 1.6b, and mozilla 1.5. Let me restate the reproducing steps: 1. Go to the URL (or coming testcase) by clicking 2. The page shows up with embedded media 3. Select File / Save Page As... 4. Make sure that File Type is "Web Page, Complete" (default anyway) 5. Click OK I have WinXP SP1, and latest WMP (ver 9.00.00.3075). Saving in view source window or saving by right-clicking at the link (without opening) works for me too. Also, if the embedded media has src= URL of "http://" instead of "mms://", it works.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
Comment 5•21 years ago
|
||
Chances are, the webbrowserpersist code sets a new src on an <object> or something along those lines, and that triggers a new load to be kicked off.... That sounds pretty wrong to me.
Comment 6•21 years ago
|
||
The problem is that WebBrowserPersist tries to fetch the mms:// URI, since it's using a protocol blacklist of things to not fetch, instead of a whitelist for things to fetch. It should either use a whitelist or not fetch things for which the protocol handler is the external protocol handler. Over to Adam.
Assignee: file-handling → adamlock
Status: UNCONFIRMED → NEW
Component: File Handling → Embedding: APIs
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Attachment #138928 -
Flags: superreview?(bz-vacation)
Comment 8•21 years ago
|
||
Comment on attachment 138928 [details] [diff] [review] draft Per email with darin, what we want here is a protocol flag that says "never returns data" and we should not persist anything with that flag....
Attachment #138928 -
Flags: superreview?(bz-vacation) → superreview-
Comment 9•21 years ago
|
||
> Per email with darin, what we want here is a protocol flag
This adds a flag to nsIProtocolHandler and uses it in nsWebBrowserPersist
Updated•21 years ago
|
Attachment #147236 -
Flags: review?(darin)
Comment 10•21 years ago
|
||
Comment on attachment 147236 [details] [diff] [review] another draft > NON_PERSISTABLE I'd rather NOT_PERSISTABLE, although my coworker notes that persistable alone is bad (it isn't a word). how about TRANSIENT or perhaps NOT_REQUESTABLE.
Comment 11•21 years ago
|
||
We're going to need this flag in more places than just persist. The real issue here is whether the protocol returns data or not.... This does leave a problem with javascript:, which returns data but still shouldn't be persisted. Similarly with view-source.... But I think that's something we may want to keep special-casing in the persistence object. On the other hand, I see no reason to not persist about: URIs or gopher: URIs. I wonder why we don't persist them now; that looks like a bug to me.
Comment 12•21 years ago
|
||
Comment on attachment 147236 [details] [diff] [review] another draft hm, why would finger and datetime not be persistable? and gnome-vfs? + * URIs for this protocol dont represent represent objects that don't nsAboutProtocolHandler::GetProtocolFlags(PRUint32 *result) { - *result = URI_NORELATIVE | URI_NOAUTH; + *result = URI_NORELATIVE | URI_NOAUTH | NON_PERSISTABLE; um, why are these not persistable? and gopher? personally I think you're adding this to way too many protocols
Comment 13•21 years ago
|
||
Comment on attachment 147236 [details] [diff] [review] another draft I agree with biesi's comments. I know that nsWebBrowserPersist blacklisted gopher and finger, but I can't think of a good reason for that. Also, I'm not sure NON_PERSISTABLE is the right choice of names for this flag. I would have used a name that better reflects the fact that it does not generate content... like: URI_NOCONTENT
Attachment #147236 -
Flags: review?(darin) → review-
Comment 14•21 years ago
|
||
OK. I'll change the name to URI_NOCONTENT and not include things like about, datetime, finger and gopher. Those had bothered me too, but I was trying to be consitent with what the current code does. The only place I guess I mistakenlly added the restriction was the gnomevfs handler. Having looked at the code, I now realize that we have a pref and restrict its usage to trusted, "data-only" protocols. So, using the flag there was totally wrong. So, like bz said, we let the flag mean "this protocol returns no data", and keep special-casing javascript and viewsource in the persist object. Right?
Comment 16•21 years ago
|
||
I think news: shouldn't have the URI_NOCONTENT flag, too. At least going to a news: URL like news://news.mozilla.org:119/36718AB4.4134F58C%40netscape.com and doing Save Page As... in Navigator works just fine currently.
Comment 17•20 years ago
|
||
is there any progress in this bug? i'm very concerned about dependent bug 181860. i visited a site with firefox 1.0 some days ago and it made me pop up mail windows until i could kill it through <img src=mailto:> tags.
Comment 18•20 years ago
|
||
should this maybe be a channel flag in some way too? that may be easier for callers to check - they can create the channel as usual, and if they don't want external content, they can check the flag and return. or, maybe, can we do something entirely different, and assume most callers do not want external content, and have asyncOpen fail unless a special load flag is set?
Comment 19•20 years ago
|
||
Hmm... The second suggestion in comment 18 has some merit if we'd done it that way to start with, but I think that would be a too-big nsIChannel API change.... Flags on the channel is doable, but again has api issues. I agree that most callers don't want no-data protocols as things stand; the one exception I'm aware of is a load in a docshell.
Updated•19 years ago
|
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
Updated•19 years ago
|
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Flags: blocking1.8.0.5?
Whiteboard: [sg:want]
Comment 21•19 years ago
|
||
Not going to make 1.8.0.5, we need some traction and a possible patch before considering this... so if a patch shows up, please nominate for the next release.
Flags: blocking1.8.0.5? → blocking1.8.0.5-
Comment 22•19 years ago
|
||
Sorry, I got swamped with other stuff to work on, so I won't, in fact, get a chance to look at this soon.
Assignee: mrbkap → nobody
QA Contact: ian → apis
Comment 23•19 years ago
|
||
Not going to block 1.8.1 for this, but we'd be happy to consider a patch that's baked on the trunk.
Flags: blocking1.8.1? → blocking1.8.1-
Updated•18 years ago
|
Flags: blocking1.9a1? → blocking1.9+
Assignee | ||
Comment 24•18 years ago
|
||
Assigning this to myself and I'll to take a look as soon as I can (tomorrow I hope). If anyone else if working on this or post a fix before me please re-assign it to yourself. Thanks!
Assignee: nobody → sciguyryan
Assignee | ||
Updated•18 years ago
|
Status: NEW → ASSIGNED
Target Milestone: mozilla1.9alpha1 → mozilla1.9alpha3
Assignee | ||
Comment 25•18 years ago
|
||
Patch v1
Attachment #138928 -
Attachment is obsolete: true
Attachment #147236 -
Attachment is obsolete: true
Attachment #256300 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 26•18 years ago
|
||
Patch v1 without the spelling error.
Attachment #256300 -
Attachment is obsolete: true
Attachment #256301 -
Flags: review?(cbiesinger)
Attachment #256300 -
Flags: review?(cbiesinger)
Comment 27•18 years ago
|
||
Comment on attachment 256301 [details] [diff] [review] Patch v1 (again) why are about, data, view-source, ldap, javascript and news not persistable? I think addbook should not have this flag either, not quite sure about that one though... also, it seems to me like you're breaking compatibility for chatzilla here - don't you need something like what it does for URI_DANGEROUS_TO_LOAD?
Assignee | ||
Comment 28•18 years ago
|
||
(In reply to comment #27) > (From update of attachment 256301 [details] [diff] [review]) > why are about, data, view-source, ldap, javascript and news not persistable? > The data protocol was recently added too the non-persistable list to fix bug 155109 (see justification there). Javascript suffers the same as I understand it, we don't really need to save in-line Javascript to a file. > I think addbook should not have this flag either, not quite sure about that one > though... I've removed it, it can always be re-added if needed (any news experts about here to answer this one?) > > also, it seems to me like you're breaking compatibility for chatzilla here - > don't you need something like what it does for URI_DANGEROUS_TO_LOAD? > Fixed, good catch.
Assignee | ||
Comment 29•18 years ago
|
||
Patch v1.1
Attachment #256301 -
Attachment is obsolete: true
Attachment #256483 -
Flags: review?(cbiesinger)
Attachment #256301 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•18 years ago
|
Attachment #256483 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 30•18 years ago
|
||
Patch v2 per the discussion on IRC.
Attachment #256483 -
Attachment is obsolete: true
Attachment #256520 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 31•18 years ago
|
||
Patch v2.1 Added the "URI_" prefix to the flag in compliance with the other flags of this type.
Attachment #256520 -
Attachment is obsolete: true
Attachment #256521 -
Flags: review?(cbiesinger)
Attachment #256520 -
Flags: review?(cbiesinger)
Assignee | ||
Updated•18 years ago
|
Attachment #256521 -
Flags: superreview?(cbiesinger)
Comment 32•18 years ago
|
||
Comment on attachment 256521 [details] [diff] [review] Patch v2.1 + NS_ENSURE_SUCCESS(rv, rv); I'm not sure this is the best way to handle this. I'd do: if (NS_FAILED(rv)) doNotPersistURI = PR_FALSE; I'd also move the variable declaration to right before the NS_URIChainHasFlags call and wouldn't initialize it. silver, this patch touches venkman/chatzilla, so you might want to look at this.
Attachment #256521 -
Flags: superreview?(cbiesinger)
Attachment #256521 -
Flags: superreview+
Attachment #256521 -
Flags: review?(silver)
Attachment #256521 -
Flags: review?(cbiesinger)
Attachment #256521 -
Flags: review+
Comment 33•18 years ago
|
||
Comment on attachment 256521 [details] [diff] [review] Patch v2.1 r=silver on the Venkman and ChatZilla parts. However, I should note that a good number of x-jsd URLs do produce content that can be saved fine; I'm not convinced this flag should be applied to that protocol.
Attachment #256521 -
Flags: review?(silver) → review+
Assignee | ||
Comment 34•18 years ago
|
||
(In reply to comment #33) > (From update of attachment 256521 [details] [diff] [review]) > r=silver on the Venkman and ChatZilla parts. > > However, I should note that a good number of x-jsd URLs do produce content that > can be saved fine; I'm not convinced this flag should be applied to that > protocol. > If you think most can be saves safely then I am fine with removing the Venkman. Biesi, any thoughts too?
Assignee | ||
Comment 36•18 years ago
|
||
Patch for checkin, updated to comments.
Attachment #256521 -
Attachment is obsolete: true
Assignee | ||
Updated•18 years ago
|
Whiteboard: [sg:want] → [sg:want] [checkin needed]
Comment 37•18 years ago
|
||
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago → 18 years ago
Resolution: --- → FIXED
Whiteboard: [sg:want] [checkin needed] → [sg:want]
Updated•18 years ago
|
Flags: in-testsuite?
Updated•17 years ago
|
Flags: wanted1.8.1.x?
Updated•17 years ago
|
Flags: wanted1.8.1.x?
Updated•17 years ago
|
Whiteboard: [sg:want]
Updated•6 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•