Closed Bug 69295 Opened 24 years ago Closed 15 years ago

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

Categories

(SeaMonkey :: Page Info, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: stas-bugzilla, Assigned: db48x)

References

Details

Attachments

(3 files, 5 obsolete files)

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.
[WISH] --> [RFE]
Summary: [WISH] Unable to follow link/copy/drag elements of Page Info, e.g. images → [RFE] Unable to follow link/copy/drag elements of Page Info, e.g. images
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
Assignee: asa → ben
Component: Browser-General → XP Apps: GUI Features
OS: Linux → All
QA Contact: doronr → sairuh
CC'ing myself. We already plan this for the links tab, I guess it wouldn't be
too hard for the images.
Marking NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
updating wrong dependancy.. yikes for the spam
Blocks: 52730
No longer blocks: 52370
Keywords: ui
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.
This is not already the case. Yay, more work for me!

db48x
-> Daniel
Assignee: ben → db48x
No longer blocks: 52730
mass moving open bugs pertaining to page info to pmac@netscape.com as qa contact.

to find all bugspam pertaining to this, set your search string to
"BigBlueDestinyIsHere".
QA Contact: sairuh → pmac
*** Bug 122124 has been marked as a duplicate of this bug. ***
Component: XP Apps: GUI Features → Page Info
*** Bug 122241 has been marked as a duplicate of this bug. ***
*** Bug 123949 has been marked as a duplicate of this bug. ***
*** Bug 129307 has been marked as a duplicate of this bug. ***
*** Bug 130312 has been marked as a duplicate of this bug. ***
It would also be nice if one could save all media objects (including embedded
such as QuickTime movies) directly from here.
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.
Status: NEW → ASSIGNED
Attached patch patch (obsolete) — Splinter Review
yay, that must have worked! :)
Attachment #74440 - Attachment is obsolete: true
Attached patch fix issues raised by caillon (obsolete) — Splinter Review
Attachment #74501 - Attachment is obsolete: true
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
Attachment #74552 - Attachment description: fix issues raised by caillion → fix issues raised by caillon
Attachment #74552 - Flags: review+
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.
Attachment #74552 - Flags: needs-work+
Attached patch updated (obsolete) — Splinter Review
Attachment #74552 - 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
weird.
/me needs sleep, ignore him.
Comment on attachment 75077 [details] [diff] [review]
updated

i'm not a fan of mixing ?: with ||. Keep or change, it's up to you.
Attachment #75077 - Flags: review+
Attachment #75077 - Flags: superreview+
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 on attachment 75077 [details] [diff] [review]
updated

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75077 - Flags: approval+
just seems like a waste of time if you ask me?

At any rate, this patch should apply much more cleanly.
Attachment #75077 - Attachment is obsolete: true
(adds some curlies that he objected to the removal of)
Attachment #77814 - Attachment is obsolete: true
Comment on attachment 77815 [details] [diff] [review]
that caillon, always nit-picking about the smallest things ;)

Carrying over r/sr/a
Attachment #77815 - Flags: superreview+
Attachment #77815 - Flags: review+
Attachment #77815 - Flags: approval+
Checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
This patch removed the Save As button.  Was that done on purpose?
no, it was quite by accident. The result of a last minute merge conflict I believe.
reopening to fix the save as blooper
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
just adds the three lines of xul back into pageInfo.xul
Comment on attachment 78157 [details] [diff] [review]
fix save as button

r=kerz
Attachment #78157 - Flags: review+
Attachment #78157 - Flags: superreview+
Comment on attachment 78157 [details] [diff] [review]
fix save as button

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

a=rjesup@wgate.com  Please check into both trunk and branch
Attachment #78157 - Flags: approval+
checked in by timeless
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
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.
Keywords: fixed1.0.0
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.
Attachment #77815 - Flags: needs-work+
And reopening...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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)
I'm making this bug dependant of bug 158593 which implements dragging from the
media and links tabs.
Depends on: 158593
Attached patch dohSplinter Review
also renames aDocument to aNode
Attachment #97204 - Flags: review+
Comment on attachment 97204 [details] [diff] [review]
doh

sr=bzbarsky
Attachment #97204 - Flags: superreview+
*** Bug 170529 has been marked as a duplicate of this bug. ***
*** Bug 171576 has been marked as a duplicate of this bug. ***
Summary: [RFE] Unable to follow link/copy/drag elements of Page Info, e.g. images → Unable to follow link/copy/drag elements of Page Info, e.g. images
Product: Browser → Seamonkey
Hardware: PC → All
(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?)
No reply to comment #48. Resetting A+QA to see if anyone is watching.
Status: REOPENED → NEW
QA Contact: pmac
>>> 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.
Status: NEW → RESOLVED
Closed: 23 years ago15 years ago
Resolution: --- → FIXED
@Philip Chee 

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

Thank you.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: