70362 - Convert PageInfo to tabbed panels

Status: VERIFIED FIXED

(SeaMonkey :: Page Info, enhancement)

Description

[Terry Hayes]

In order to allow other modules to add content to the pageinfo display, the
basic structure should be changed to provide tabs for accessing specific
information.   Hooks should be provided to allow modules to add panels by
overlay and to call javascript in the overlay when the page info is displayed.

Comment 1

[Terry Hayes]

Attachment: Patches to pageInfo.dtd - add tab labels

Comment 2

[Terry Hayes]

Attachment: Patches to pageInfo.xul - add tabs for content

Comment 3

[Terry Hayes]

Attachment: Patches to pageInfo.js - add onload registry and initial tab selection

Comment 4

[timeless]

+   if (window.arguments != null) {
use
if ("arguements" in window)
other comparisons (a != null) can be written as (a)

ps what's xcollapsed?

Comment 5

[Jesse Ruderman]

I haven't tried the patch, but what happens if a page doesn't have forms -- is 
the forms tab disabled, removed, or left there?  What should happen?

Comment 6

[Fabian Guisset]

Ack there is a problem. Daniel Brooks is already working on the new page info
and has all of this working plus much more. The tracking bug is bug 52730, but
we haven't added anything in that bug yet. The latest snaphshot of the new Page
Info can be found at db48x.hypermart.net/pageinfo/

Comment 7

[Daniel Brooks [:db48x]]

Oh..., that's cool. That'll let you show any tab to start out with. Not sure
exactly how you propose to go about lettign other modules add their own tabs,
but I'll have to look up how overlays work, seems like that's the key. 

As for duplication of work, well, there isn't really much in the way of that
here. I can let you patch that part in as soon as everything else is done, or we
can combine the two, whichever. Easiest and cleanest for you to make your own
patch, as you've done here, but against whatever finally gets used elsewhere.
/me hurridly adds ids to all the tabs... :)

Comment 8

[Terry Hayes]

Attachment: Example of overlay to add module-specific info

Comment 9

[Terry Hayes]

Attachment: New patches to pageInfo files - hidden tabs, general cleanup

Comment 10

[Terry Hayes]

I'd like to get review on these patches to enable the security work.  I believe
that bug 52730 can build on these basic changes, which adds the following:
  + tabs
  + selection of tab at window open
  + overlay capability for other modules
  + registration of onLoad handler for other modules

Please review and approve.  Who should super-review?

Comment 11

[Daniel Brooks [:db48x]]

looks good. only one thing: it crashes patch on my machine. probably nothing,
I'm probably doing something wrong.

Comment 12

[Matthew T (active 1999-2002)]

I'm curious. Can you give a couple of examples of modules, which would add tabs 
other than those already specified in the spec for bug 52730?

Comment 13

[Daniel Brooks [:db48x]]

Theses changes are for psm. The security tab will be done with an overlay.
That's the only module I know of at the moment, but any of them could add to it
if they like.

Comment 14

[Terry Hayes]

I don't have any additions to the list.  The reason for doing this work now is
to enable us to add the Security tab without depending on work for other
features in the pageinfo window.  In addition, the code for providing
information about security is best placed in the security module itself, which
is why these changes are designed to provide the overlay capabilities.

I'd rather see a framework put in place sooner, rather than wait for a single
landing that provides everything in one (large) implementation file.

Comment 15

[Paul Chen]

Well, just applied this to my mac tree. Looks ok. r=pchen

Comment 16

[Alec Flett]

sr=alecf - looks cool!

Comment 17

[Blake Ross]

This was fixed, right?  Can we close this?

Comment 18

[sairuh (rarely reading bugmail)]

yep, page info now has the tabbed ui.

Comment 19

[Terry Hayes]

Attachment: Patches for merge conflict and XUL changes

Comment 20

[Terry Hayes]

New patch (attached) to fix a merge error (within 1 hour of my previous
changes).  Please ignore the XUL changes.

Request review (pchen) and super-review (alecf).

Comment 21

[Alec Flett]

What issue does this patch address?

Please don't check in the pageInfo.xul change - you don't need the .pageinfo

as for the window.arguments thing, care to explain why you need to change that? 

Comment 22

[Terry Hayes]

pageInfo.xul will not be checked in.

My original change to pageInfo.js added an optional argument to this dialog that
specified the id of the tab to select.  The idea is to use this  argument for
selecting the security tab when this window is opened from the lock icon.  The
tab name was arguments[0] in the original code.

1 hour after my checkin, someone added a different use for arguments[0].  I'm
resolving this conflict by moving the tab name to arguments[1].

Comment 23

[Alec Flett]

thanks for the explanation - good to have these things recorded somewhere :)
sr=alecf (and thanks for the clearer comment in the patch, as well!)

Comment 24

[Daniel Brooks [:db48x]]

yea, this is needed for bug 70632. I was going to fix it a different way though.
At any rate, this is the most straitforward way to fix it now.

Comment 25

[Terry Hayes]

Fix in last patch checked in.

Comment 26

[sairuh (rarely reading bugmail)]

vrfy.