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

RESOLVED FIXED

Status

enhancement
RESOLVED FIXED
19 years ago
10 years ago

People

(Reporter: stas-bugzilla, Assigned: db48x)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 5 obsolete attachments)

Reporter

Description

19 years ago
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

19 years ago
[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

Comment 3

19 years ago
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

Comment 4

19 years ago
updating wrong dependancy.. yikes for the spam
Blocks: 52730
No longer blocks: 52370

Updated

19 years ago
Keywords: ui
Assignee

Comment 5

19 years ago
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.
Assignee

Comment 6

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

db48x

Comment 7

19 years ago
-> Daniel
Assignee: ben → db48x

Updated

18 years ago
Blocks: 82059

Updated

18 years ago
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
Assignee

Comment 9

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

Updated

18 years ago
Component: XP Apps: GUI Features → Page Info
Assignee

Comment 10

18 years ago
*** Bug 122241 has been marked as a duplicate of this bug. ***
Assignee

Comment 11

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

Comment 12

18 years ago
*** Bug 129307 has been marked as a duplicate of this bug. ***
*** Bug 130312 has been marked as a duplicate of this bug. ***

Comment 14

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

Comment 15

18 years ago
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
Assignee

Comment 16

18 years ago
Posted patch patch (obsolete) — Splinter Review
yay, that must have worked! :)
Assignee

Comment 17

18 years ago
Attachment #74440 - Attachment is obsolete: true
Assignee

Comment 18

18 years ago
Posted 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 20

18 years ago
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+
Assignee

Comment 21

18 years ago
Posted patch updated (obsolete) — Splinter Review
Attachment #74552 - Attachment is obsolete: true
Assignee

Comment 22

18 years ago
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.
Assignee

Comment 23

18 years ago
/me needs sleep, ignore him.

Comment 24

18 years ago
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+

Updated

18 years ago
Attachment #75077 - Flags: superreview+

Comment 25

18 years ago
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+
Assignee

Comment 27

18 years ago
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
Assignee

Comment 28

18 years ago
(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: 18 years ago
Resolution: --- → FIXED

Comment 31

18 years ago
This patch removed the Save As button.  Was that done on purpose?
Assignee

Comment 32

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

Comment 33

18 years ago
reopening to fix the save as blooper
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 34

18 years ago
just adds the three lines of xul back into pageInfo.xul

Comment 35

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

r=kerz
Attachment #78157 - Flags: review+

Updated

18 years ago
Attachment #78157 - Flags: superreview+

Comment 36

18 years ago
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+
Assignee

Comment 38

18 years ago
checked in by timeless
Status: REOPENED → RESOLVED
Closed: 18 years ago18 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 → ---
Assignee

Comment 42

17 years ago
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
Assignee

Comment 44

17 years ago
Posted patch dohSplinter Review
also renames aDocument to aNode

Updated

17 years ago
Attachment #97204 - Flags: review+
Comment on attachment 97204 [details] [diff] [review]
doh

sr=bzbarsky
Attachment #97204 - Flags: superreview+

Comment 46

17 years ago
*** Bug 170529 has been marked as a duplicate of this bug. ***
*** Bug 171576 has been marked as a duplicate of this bug. ***

Updated

17 years ago
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

Updated

13 years ago
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

Comment 50

10 years ago
>>> 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: 18 years ago10 years ago
Resolution: --- → FIXED

Comment 51

10 years ago
@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.