69295 - Unable to follow link/copy/drag elements of Page Info, e.g. images

Status: RESOLVED FIXED

(SeaMonkey :: Page Info, enhancement)

Description

[Stanislav Malyshev]

When I open Page Info on a page with image, I'd like to be able to operate
somehow on the images. For example: to be able to view it, to copy it's URL, to
drag it out. Unfortunately, it's impossible neither from "Images on" list, nor
from image view when clicked. Enabling this capability would be good.

Comment 1

[Oliver Klee]

[WISH] --> [RFE]

Comment 2

[Boris Zbarsky [:bzbarsky]]

See bug 55713 for copying from page info.

Also see bug 52730 for general work on page info.

Ccing the contributor working on new page info dialog, and over to XP Apps:GUI
Features

Comment 3

[Fabian Guisset]

CC'ing myself. We already plan this for the links tab, I guess it wouldn't be
too hard for the images.
Marking NEW.

Comment 4

[Fabian Guisset]

updating wrong dependancy.. yikes for the spam

Comment 5

[Daniel Brooks [:db48x]]

You should also be able to right click it in the preview and get some standard 
context menu items for images, in my opinion. Have to check if this is the case 
when I get home.

Comment 6

[Daniel Brooks [:db48x]]

This is not already the case. Yay, more work for me!

db48x

Comment 7

[Blake Ross]

-> Daniel

Comment 8

[sairuh (rarely reading bugmail)]

mass moving open bugs pertaining to page info to [e-mail] as qa contact.

to find all bugspam pertaining to this, set your search string to
"BigBlueDestinyIsHere".

Comment 9

[Daniel Brooks [:db48x]]

*** Bug 122124 has been marked as a duplicate of this bug. ***

Comment 10

[Daniel Brooks [:db48x]]

*** Bug 122241 has been marked as a duplicate of this bug. ***

Comment 11

[Daniel Brooks [:db48x]]

*** Bug 123949 has been marked as a duplicate of this bug. ***

Comment 12

[Alfonso Martinez]

*** Bug 129307 has been marked as a duplicate of this bug. ***

Comment 13

[Boris Zbarsky [:bzbarsky]]

*** Bug 130312 has been marked as a duplicate of this bug. ***

Comment 14

[Moritz Balz]

It would also be nice if one could save all media objects (including embedded
such as QuickTime movies) directly from here.

Comment 15

[Daniel Brooks [:db48x]]

I have a patch for this, but I won't even attach it until bugs 121567, 121899
and 122055 are checked in. too much work for me to maintain so many different
versions of the same two files.

Comment 16

[Daniel Brooks [:db48x]]

Attachment: patch

yay, that must have worked! :)

Comment 17

[Daniel Brooks [:db48x]]

Attachment: sane patch, fix issues raised by timeless

Comment 18

[Daniel Brooks [:db48x]]

Attachment: fix issues raised by caillon

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 74552 [details] [diff] [review]
fix issues raised by caillon

Looks fine, although I still think this could be re-written with a constant
object var...  But since you already have another instance of similar code in
the file, I won't force it.

>+function grabAllXLinks(aDocument)
>+{
>+  function XLinkFilter() 
>+  {
>+    this.acceptNode = function(aDocument)
>+    {
>+      return (aDocument.hasAttributeNS(XLinkNS, "href")) ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP;
>+    }     
>+  }
>+
>+  var nodeFilter = new XLinkFilter;

Assuming it's been thoroughly tested, r=caillon

Comment 20

[Alec Flett]

Comment on attachment 74552 [details] [diff] [review]
fix issues raised by caillon

dont' call it nsIClipboardHelper, that's the name of an interface, not a class.
Besides, it's global. You should call it gClipboardHelper at the very least.

I'm a bit confused about these magic numbers passed to the "copycol" parameter
of the pageInfoOutlinerView constructor. could we at least move these to
"const" declarations at the top of the page (as in:

const COPY_COLUMN_NONE = -1;
const COPY_COLUMN_NAME = 1;

etc.. otherwise, the callers of the constructor don't have any obvious
documentation.

Comment 21

[Daniel Brooks [:db48x]]

Attachment: updated

Comment 22

[Daniel Brooks [:db48x]]

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
weird.

Comment 23

[Daniel Brooks [:db48x]]

/me needs sleep, ignore him.

Comment 24

[timeless]

Comment on attachment 75077 [details] [diff] [review]
updated

i'm not a fan of mixing ?: with ||. Keep or change, it's up to you.

Comment 25

[Alec Flett]

Comment on attachment 75077 [details] [diff] [review]
updated

much nicer. a few comments here and there get you bonus points, bug sr=alecf
anyway.

Comment 26

[Asa Dotzler [:asa]]

Comment on attachment 75077 [details] [diff] [review]
updated

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 27

[Daniel Brooks [:db48x]]

Attachment: why did they have to rename outliners to trees anyway?

just seems like a waste of time if you ask me?

At any rate, this patch should apply much more cleanly.

Comment 28

[Daniel Brooks [:db48x]]

Attachment: that caillon, always nit-picking about the smallest things ;)

(adds some curlies that he objected to the removal of)

Comment 29

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 77815 [details] [diff] [review]
that caillon, always nit-picking about the smallest things ;)

Carrying over r/sr/a

Comment 30

[Christopher Aillon (sabbatical, not receiving bugmail)]

Checked in

Comment 31

[Neil Marshall]

This patch removed the Save As button.  Was that done on purpose?

Comment 32

[Daniel Brooks [:db48x]]

no, it was quite by accident. The result of a last minute merge conflict I believe.

Comment 33

[Daniel Brooks [:db48x]]

reopening to fix the save as blooper

Comment 34

[Daniel Brooks [:db48x]]

Attachment: fix save as button

just adds the three lines of xul back into pageInfo.xul

Comment 35

[Jason Kersey]

Comment on attachment 78157 [details] [diff] [review]
fix save as button

r=kerz

Comment 36

[Alec Flett]

Comment on attachment 78157 [details] [diff] [review]
fix save as button

sr=alecf

Comment 37

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 78157 [details] [diff] [review]
fix save as button

a=[e-mail]  Please check into both trunk and branch

Comment 38

[Daniel Brooks [:db48x]]

checked in by timeless

Comment 39

[Asa Dotzler [:asa]]

adding fixed1.0.0 keyword (branch resolution). This bug has comments saying it
was fixed on the 1.0 branch and a bonsai checkin comment that agrees. To verify
the bug has been fixed on the 1.0 branch please replace the fixed1.0.0 keyword
with verified1.0.0.

Comment 40

[Johnny Stenback (:jst)]

Comment on attachment 77815 [details] [diff] [review]
that caillon, always nit-picking about the smallest things ;)

The function: XLinkFilter.acceptNode that was checked in is broken, here's the
function:

+function grabAllXLinks(aDocument)
+{
+  function XLinkFilter() 
+  {
+    this.acceptNode = function(aDocument)
+    {
+      return (aDocument.hasAttributeNS(XLinkNS, "href")) ?
NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP;
+    }	   
+  }
+

a) There's no need to create a function with a acceptNode property on it, just
create a normal function (XLinkFilter) and pass that function as the filter to
the tree walker.

b) DOM document's don't have hasAttributeNS() methods, so this gives errors on
the JS console.

+  var nodeFilter = new XLinkFilter;
+  var iterator = aDocument.createTreeWalker(aDocument,
NodeFilter.SHOW_ELEMENT, nodeFilter, true);

c) Just pass XLinkFilter to the createTreeWalker method, no need to new an
XLinkFilter object.

d) You're asking the tree walker to only show elements, yet the name of the
argument to the filter suggests that you want the document to be passed to the
filter, not the elements.

Comment 41

[Johnny Stenback (:jst)]

And reopening...

Comment 42

[Daniel Brooks [:db48x]]

a) I guess that's a matter of style
b) I didn't know that (and hadn't noticed any JS errors, maybe those are more 
recent.) Easily fixed though.
c) Actually, the last time I tried that, mozilla crashed. Funny thing was, it 
worked fine like you suggest just a few hundred lines above that example 
(FindFirstControl). I suppose the moon was just wrong, or I wasn't holding my 
mouth right, but I haven't tried it since.
d) You are correct, I haven't the foggiest why that varible is called 
aDocument. Must've made sense at the time. Anyway another simple fix. (I bet I 
used the same varible name in FindFirstControl)

Comment 43

[Nicolás Lichtmaier]

I'm making this bug dependant of bug 158593 which implements dragging from the
media and links tabs.

Comment 44

[Daniel Brooks [:db48x]]

Attachment: doh

also renames aDocument to aNode

Comment 45

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 97204 [details] [diff] [review]
doh

sr=bzbarsky

Comment 46

[Jo Hermans]

*** Bug 170529 has been marked as a duplicate of this bug. ***

Comment 47

[Mats Palmgren (inactive)]

*** Bug 171576 has been marked as a duplicate of this bug. ***

Comment 48

[Tony Mechelynck [:tonymec]]

(In reply to comment #44)
> Created an attachment (id=97204) [details]
> doh
> 
> also renames aDocument to aNode

This patch has r+ sr+ since 2002 but IIUC it was never landed.
- Anyone still desires to see this bug fixed? (Stanislav?)
- Daniel, are you willing to try and un-bitrot the patch? (No time, maybe?)

Comment 49

[Tony Mechelynck [:tonymec]]

No reply to comment #48. Resetting A+QA to see if anyone is watching.

Comment 50

[Philip Chee]

>>> Created an attachment (id=97204) [details] [details]
> This patch has r+ sr+ since 2002 but IIUC it was never landed.

This last patch was obsoleted by the pageInfo rewrite. Marking as FIXED as there are no remaining issues.

Comment 51

[Andrew Hagen]

@Philip Chee 

Was the the Page Info rewrite bug 339102? If not, could you please post the bug number?

Thank you.