Closed Bug 83017 Opened 23 years ago Closed 22 years ago

Frame Info dialog has title 'View Page Info'

Categories

(SeaMonkey :: Page Info, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: ajbu, Assigned: db48x)

References

()

Details

Attachments

(1 file, 9 obsolete files)

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows NT 5.0)
BuildID:    2001052720

The Frame Info dialog has title 'View Page Info'. This can be confusing when 
looking for frame info, making you think the wrong option was picked.

Reproducible: Always
Steps to Reproduce:
1. Go to a site with frames. http://www.macromedia.com
2. Click with the right mouse button on the page.
3. From the two info option 'View Page Info' and View Frame Info' choose 'View 
Frame Info' (both with the 'i' as accellerator key)

Actual Results:  The Frame info dialog has the title 'Page Info' and the 
header 'Information about the current page'

Expected Results:  The Frame Info dialog should be called 'Frame Info' with the 
header 'Information about the current frame'
Hmmm...  A typo.  Sharp eye, ajbu@planet.nl!!
Status: UNCONFIRMED → NEW
Ever confirmed: true
-> Daniel
Assignee: blakeross → db48x
Depends on: 76250
hi, I'm working on the MultiZilla project and like to know if something like
this could work:

pageInfo.xul
<text id="pageHeader1" class="header label" value="&pageInfo.description;"/>
<text id="frameHeader1" hidden="true" class="header label"
value="&frameInfo.description;"/>

<text id="pageHeader2" class="header label"
value="&pageInfo.description;"/><text id="frameHeader2"  hidden="true"
class="header label" value="&frameInfo.description;"/>

and add these lines to pageInfo.dtd
<!ENTITY  frameInfo.title "Frame Info">
<!ENTITY  frameInfo.description "Information about the current frame"> 

and add some logic to pageInfo.js like:

if (window.frames.length) {
  document.getElementById("pageHeader1").setAtribute("hidden","true");
  document.getElementById("pageHeader2").setAtribute("hidden","true");
  document.getElementById("frameHeader1").setAtribute("hidden","false");
  document.getElementById("frameHeader2").setAtribute("hidden","false");
}

I didn't test this, I just talked this true with HJ: bugs4hj@netscape.net
If it doesn't work, don't blame him, blame me, HJ is still hospitalized.
CC'ed, just spam.
That won't necessarily work, because you could be asking for page info on a page
with multiple frames, or frame info for a frame with subframes. The very easiest
way to do it is to add another parameter to the arguments and have it indicate
whether it's for frame info or page info, (set to 1 for frame info, unset or set
to 0 for page info). bug 76250 will still need to be fixed, and I'll go ahead
and change the way arguments are passed to be a little more robust when I fix it
(I was going to fix this one at the same time I did the other, but oh well...)
Attached patch should fix it (obsolete) — Splinter Review
oh, wait a bit, I'm dumb. should have slept last night I guess
Great, but what about "Information about the current page" is that lost now?
somewhere there is another bug to remove it, so I went ahead and removed it. It
was kinda obvious that the dialog had information about the page, wasn't it?
Severity: trivial → minor
Status: NEW → ASSIGNED
OS: Windows 2000 → All
Hardware: PC → All
Target Milestone: --- → mozilla0.9.4
looks like this one is close.  try for 0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5
r=choess
What exactly is this supposed to do?

 function onLoadPageInfo()
 {
+  var strbundle = document.getElementById("pageinfobundle");
+  window.title = title;
I don't see title being defined anywhere, 'title' is a global property (i.e.
window.title), but the code "window.title = window.title" doesn't make any sense
to me. Besides, window.title is to be deprecated so don't use that, you should
use document.title in stead of window.title.

Take care of that and I'll have one more look before giving my sr=
Daniel, there's something I don't understand... the stringbundle refers to
pageInfo.properties (as it should), but you didn't add any text entry to it.. it
only contains unknown=Unknown as far as I know. You cannot get strings from a
dtd file.
Also please rename var title to something like var docTitle because it can be
easily confused with the global var window.title (which can be accessed using
|title|).
Daniel, please use "getBrowser().contentDocument.title" for the title to improve
future integration with other software like MultiZilla.

Thank you.
Neil Pryde
Fabian: pageInfo.properties didn't exist when I created the first patches. (And
I can't create file, no write perms of course) Thanks for the catch.
Neil: lemme test that, just to make sure...
Attached patch last patch perhaps? (obsolete) — Splinter Review
Neil: I knew there was a reason getBrowser().contentDocument.title felt weird.
I'm not trying to change the title of the main browser window, I'm changing the
title of my own little page info window. Thanks though.
Attached patch this is the right one (obsolete) — Splinter Review
Comment on attachment 48566 [details] [diff] [review]
this is the right one

r=fabian good job ;-)
Attachment #48566 - Flags: review+
Comment on attachment 48566 [details] [diff] [review]
this is the right one

sr=jst
Attachment #48566 - Flags: superreview+
Er, me bad, sorry for the confusion (:
0.9.5 is out the door. bumping TM up by one.
Target Milestone: mozilla0.9.5 → mozilla0.9.6
can this get checked in before the freeze?
Checking in xpfe/browser/jar.mn;
/cvsroot/mozilla/xpfe/browser/jar.mn,v  <--  jar.mn
new revision: 1.26; previous revision: 1.25
done
Checking in xpfe/browser/resources/content/pageInfo.js;
/cvsroot/mozilla/xpfe/browser/resources/content/pageInfo.js,v  <--  pageInfo.js
new revision: 1.13; previous revision: 1.12
done
Checking in xpfe/browser/resources/content/pageInfo.xul;
/cvsroot/mozilla/xpfe/browser/resources/content/pageInfo.xul,v  <--  pageInfo.xul
new revision: 1.21; previous revision: 1.20
done
Checking in xpfe/browser/resources/locale/en-US/pageInfo.dtd;
/cvsroot/mozilla/xpfe/browser/resources/locale/en-US/pageInfo.dtd,v  <-- 
pageInfo.dtd
new revision: 1.5; previous revision: 1.4
done
Checking in xpfe/browser/resources/locale/en-US/pageInfo.properties;
/cvsroot/mozilla/xpfe/browser/resources/locale/en-US/pageInfo.properties,v  <--
 pageInfo.properties
new revision: 1.2; previous revision: 1.1
done

Manifest not checked in, it's a dead file.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Um.. what got checked in does not match the latest patch in this bug.  In
particular, no all occurrences of &pageInfo.description; got removed from the
XUL, causing bug 108677
*** Bug 108685 has been marked as a duplicate of this bug. ***
Sorry, this was the result of a merge conflict
Checking in pageInfo.xul;
/cvsroot/mozilla/xpfe/browser/resources/content/pageInfo.xul,v  <--  pageInfo.xul
new revision: 1.22; previous revision: 1.21
done
*** Bug 108758 has been marked as a duplicate of this bug. ***
*** Bug 108895 has been marked as a duplicate of this bug. ***
unable to verify this coz Page/Frame Info is broken in the 11/6 bits i have.
spoke w/timeless who sez this is now fixed, but i'll need to wait for a more
stable set of verification builds to test this one...
*** Bug 109035 has been marked as a duplicate of this bug. ***
now all Info windows have 'Frame Info' in the titlebar --even when viewing a
plain ole non-framed page [eg, http://mozilla.org]. found using
2001.11.08.0x-comm bits on all platforms.

so, reopening.

in addition, Page Info from the context menu always displays info for the active
frame --filed bug 109118.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
unfortunatly, I'm not seeing that here on my build. I'll keep looking.
*** Bug 109398 has been marked as a duplicate of this bug. ***
Ok, I figued out why this went so badly. Somewhere along the line, someone
started passing the arguments to the Page Info window all wrong when they opened
it. It's opened like this in navigator.js:

  window.openDialog("chrome://navigator/content/pageInfo.xul",
                    "_blank",
                    "chrome,dialog=no",
                    doc);

and like this in securityUI.js:

   window.openDialog("chrome://navigator/content/pageInfo.xul", "_blank",
                     "dialog=no", null, "securityTab");

As you can see, the call in navigator.js passes the frame to show info for,
while the one in securityUI.js passes in the name of the tab it wants selected.

I could have sworn that I'd filed a bug to remind me to clean up the arguments
and stuff, but I can't find it anywhere. I suppose I should just file another...

Anyway, don't ask me how/if it ever worked before...
In order for page info/frame info to function, it must have access to the
DOMWindow of the document that you want information to. Currently, all that is
passed in is the document itself. Unfortunatly, there isn't really a way to get
the window from the document (though you can go the other way quite easily), so
I'm not sure frame info is possible. From my reading of the context menu handler
(the only way to activate frame info is from a context menu), there is no way
for it to know what window it's in either, just the document.

The only way out that I can think of is for the context menu handler to it's
window.frames to find the frame containing the document the right click was in,
and either pass that window object, or just pass the index of it, and I can get
it via window.opener.frames, either way it'll be slower. Also, would I be able
to compare two document objects like that? (doc1 == doc2?) I know we don't yet
support the DOM3 methods for comparing nodes.

Any comments?
Status: REOPENED → ASSIGNED
oh, either that or we could add a property to all DOMDocuments called and have
it point to the window the document is in, much like DOMWindows have a property
called document that points to the document they show...
Whiteboard: impossible
On the other hand, as Fabian suggests on irc, perhaps by selecting Frame Info
over Page Info, the user doesn't care to see all the information on the
subframes the way Page Info shows. If so then it's easy enough just to not look
for that information, which was the only reason it needed the window object in
the first place.
Depends on: 82059
then again, fabian discovered that the __parent__ property is the window object
I seek, so all we have to do is decide if we should be showing the images,
links, etc from the subframes (should they exist) or not.

Before now I had assumed that's what we should do, but now I'm not so sure.
Anyone care to comment?
I have the fix for this in my tree, but it's intertwined with bug 52730. Should
I back-port it to the older files like my original fix for this bug, or should I
try get bug 52730 checked in before .9.6 is released? (do we even care if this
is fixed prior to .9.6?)

fixing this bug requires changes to navigator.js, and maybe one or two other
places (have to check again), bug 52730 does not unless it also fixes this bug.

both bugs will need r=, sr= and a=, bug 52730 is bigger and harder to review.

Asa, your opinion?
These are the changes required to make everyone that opens the page info pass
the correct arguments. Does not include any changes to pageinfo.js itself,
though a few minor ones are needed. If need be, I can seperate them from bug
52730, though currently I don't think it'll be necessary; it shouldn't be too
long till 52730 is checked in.
Attachment #46708 - Attachment is obsolete: true
Attachment #46709 - Attachment is obsolete: true
Attachment #46711 - Attachment is obsolete: true
Attachment #46934 - Attachment is obsolete: true
Attachment #48549 - Attachment is obsolete: true
Attachment #48562 - Attachment is obsolete: true
Attachment #48564 - Attachment is obsolete: true
Attachment #48565 - Attachment is obsolete: true
Attachment #48566 - Attachment is obsolete: true
0.9.6 has gone the way of 0.9.5 (i.e., released without this).
0.9.6 is long gone. -> 0.9.7
Target Milestone: mozilla0.9.6 → mozilla0.9.7
*** Bug 117024 has been marked as a duplicate of this bug. ***
this needs to be checked in, it's a rather silly bug. ->0.9.8
Whiteboard: impossible
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Attachment #57555 - Flags: review+
As of build 2002010103, the View Page Info window says "Frame Info", which seems
to be the inverse of what this bug is talking about.

I wouldn't worry about it too much now, though... isn't it all going to be
superseded soon when bug 52730 -- the complete rewrite of the Page Info window
-- finally lands?
this bug has morphed slightly since then, the patch in this bug is complementary
with the one in 52730 - they both need to be checked in.
Depends on: 120043
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
Component: XP Apps: GUI Features → Page Info
Daniel, I think this should nominate nsbeta1, what do you think? 
Keywords: nsbeta1
yes, I agree. it's extremely annoying. Especially since I caused it in the first
place :)
*** Bug 127500 has been marked as a duplicate of this bug. ***
Daniel what is left to do here? Tell me when you have a patch I have to review,
please. Changing target milestone to a more realistic one
Target Milestone: mozilla0.9.8 → mozilla1.0
Just have to get the last attachment super reviewed and checked in. I last sent
an email to some sr's and reviewers@mozilla Friday morning.
nsbeta1- per ADT triage. 
Keywords: nsbeta1nsbeta1-
Keywords: mozilla1.0
Comment on attachment 57555 [details] [diff] [review]
diff with changes to files other than pageinfo.js itself

sr=alecf
Attachment #57555 - Flags: superreview+
Comment on attachment 57555 [details] [diff] [review]
diff with changes to files other than pageinfo.js itself

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #57555 - Flags: approval+
I checked in the patch for Daniel.
fixed
Status: ASSIGNED → RESOLVED
Closed: 23 years ago22 years ago
Resolution: --- → FIXED
Verified on netscape trunk build: 2002-12-04-08-TRUNK)
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: