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

RESOLVED FIXED

Status

enhancement
RESOLVED FIXED
18 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

18 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

18 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

18 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

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

Updated

18 years ago
Keywords: ui
(Assignee)

Comment 5

18 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

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

db48x

Comment 7

18 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

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

Comment 12

17 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

17 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

17 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

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

Comment 17

17 years ago
Attachment #74440 - Attachment is obsolete: true
(Assignee)

Comment 18

17 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

17 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

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

Comment 22

17 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

17 years ago
/me needs sleep, ignore him.

Comment 24

17 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

17 years ago
Attachment #75077 - Flags: superreview+

Comment 25

17 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

17 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

17 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
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 31

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

Comment 32

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

Comment 33

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

Comment 34

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

Comment 35

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

r=kerz
Attachment #78157 - Flags: review+

Updated

17 years ago
Attachment #78157 - Flags: superreview+

Comment 36

17 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

17 years ago
checked in by timeless
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 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

12 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
Last Resolved: 17 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.