[RFE] Should be able to save images from page info dialog box



Page Info
16 years ago
13 years ago


(Reporter: Neil Marshall, Assigned: Neil Marshall)


Firefox Tracking Flags

(Not tracked)



(1 attachment, 2 obsolete attachments)



16 years ago
Created attachment 74204 [details] [diff] [review]
Adds a Save As... button to the media tab

db48x, would you mind telling me how I should handle the no image selected
case?  I want to grey out the button, but I have no idea how I would go about
doing that.

Right now it fires off an exception to the javascript console.
the most straightforward way would be to look at the onselect handler for that
outliner (onImageSelect()), and make it disable the button when there are 0
items selected, and enable it when there is an item selected. Just add an else
clause to that if and so on.
actually, while you're at it, pull your call to the saveURL function out into a
simple function (call it 'save' or something) in pageInfo.js, so that I can have
the context menu call the same one. have it accept a single argument for the url.
also, it might be better to take the button out of the grid. try putting it in
an hbox between the grid and the preview box, and adding a spacer so that the
button is right justified. which do you like better, left or right justified?
Either way we should take it out of the grid.

Comment 4

16 years ago
I've run into a problem.  When I move the save function out of the xul and into 
the js file I get this Console error.

Warning: reference to undefined property this.parentNode.updateCurrentBrowser
(About 11 times)

As for the button, I was thinking of right justifying it when I was first going 
to add it, but never did for some reason :)

Comment 5

16 years ago
Created attachment 74239 [details] [diff] [review]
Adds the save as button to the dialog

Okay here is the updated version.  The button enables and disables itself
properly.  I tried the button on the right, but it just didn't look right.

I still can't figure out if the javascript errors I'm seeing are from my patch
or not.  It's intermittant too.  Sometimes I don't get any errors, sometimes I
get a screenfull.

Oh, thinking about this even more, perhaps there should be some sort of toolbar
where you can block images as well as saving them.  And if you have banner
blinds installed, block them based on their width and height.
Attachment #74204 - Attachment is obsolete: true
Comment on attachment 74239 [details] [diff] [review]
Adds the save as button to the dialog

it's better to remove the disabled attribute rather than set it to false, but
whichever. r=db48x
Attachment #74239 - Flags: review+

Comment 7

16 years ago
If I remove the disabled="true" from the button, then when viewing about:blank, 
the button is enabled when it shouldn't be.

Comment 8

16 years ago
IMHO dupe of 110075 (regarding its summary).
Adam: they're seperate bugs. related though.
Neil, do you mind if I reassign this to you?
Assignee: db48x → neil.marshall
At 12:06 AM 3/15/02 -0500, you wrote:
>I found a problem with the save image code that I just wrote...
><http://bugzilla.mozilla.org/show_bug.cgi?id=131031> It doesn't work
>with non-images because I was just reading the .src, which returns a
>relative url when checking relativly embeded flash/quicktime files.
>When I pass the relative url to the getAbsoluteURL function like this:
>function saveMedia(rawurl) {
>  url = getAbsoluteURL(rawurl.href || rawurl.src, rawurl);
>  [...]
>I get the relative url tacked on to a chrome:// url which is not what I
>was looking for at all.  What I want is the
>document.getElementById('embedtag').baseURI minus the filename at the
>end.  I've been staring at the getAbsoluteURL code for an hour or so and
>I still can't figure it out.  What exactly do you want sent in for the
>node paramater?
>Am I just not passing in the proper information, or is there a bug
>inside the function? 

rather than passing the url to saveMedia(), pass the media object itself.
getAbsoluteURL() expects this as the second parameter:

function saveMedia(item)
  url = getAbsoluteURL(item.href || item.src, item);
  saveURL(url, null, 'SaveImageTitle', false );

good thing you caught that, I'd forgotten all about the issue. mental block I
guess :)

Comment 12

16 years ago
Comment on attachment 74239 [details] [diff] [review]
Adds the save as button to the dialog

Attachment #74239 - Flags: superreview+

Comment 13

16 years ago
Created attachment 75110 [details] [diff] [review]
Updated version

This new version works for all media.  I also removed some strict warnings.
Attachment #74239 - Attachment is obsolete: true
I suppose that looks good enough, but if you don't mind me asking, why move
stuff out of the if in onImageSelect() and into a seperate funciton? Just seems

Comment 15

16 years ago
Because I needed to use essentially the same code in the saveMedia function.  
Why duplicate it?
Comment on attachment 75110 [details] [diff] [review]
Updated version

r=db48x, now that I see the second call to getSelectedItem()
Attachment #75110 - Flags: review+

Comment 17

16 years ago
Comment on attachment 75110 [details] [diff] [review]
Updated version

would be cool if we could someday save multiple images at a time, but that's
beyond the scope of this bug :)
Attachment #75110 - Flags: superreview+

Comment 18

16 years ago
Comment on attachment 75110 [details] [diff] [review]
Updated version

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75110 - Flags: approval+
checked in
Last Resolved: 16 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.