119858 - "p3pv1" link in Site Navigation toolbar?

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Henrik Gemal]

If you go to http://gemal.dk a "p3pv1" link appear in Site Navigation toolbar as
the last item in the "More" category. Should there be a link in the Site
Navigation toolbar to the p3p xml file? Mozilla cant even show the XML file?

Shouldn't Mozilla for now just ignore this:
<link rel="P3Pv1" href="w3c/p3p.xml">
?

build 20020111

Comment 1

[Peter Trudelle]

->[e-mail]

Comment 2

[Christopher Hoess (gone)]

We should be filtering schema* and p3p* in rel attributes, but that got taken out 
in one of the rewrites.  I'll see if I can work up a patch this evening.

Comment 3

[Christopher Hoess (gone)]

Attachment: Changed data structures a little, for convenience in solving this.

Comment 4

[Christopher Hoess (gone)]

Attachment: Fixed patch per timeless

Comment 5

[Christopher Hoess (gone)]

Comment on attachment 64984 [details] [diff] [review]
Fixed patch per timeless

>Index: linkToolbarHandler.js
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/browser/resources/content/linkToolbarHandler.js,v
>retrieving revision 1.3
>diff -u -r1.3 linkToolbarHandler.js
>--- linkToolbarHandler.js	2002/01/09 03:02:36	1.3
>+++ linkToolbarHandler.js	2002/01/15 06:43:48
>@@ -65,9 +65,8 @@
>     linkToolbarUI.activate();
>   }
> 
>-  var relAttributes = linkElement.rel.split(" ");
>-  for (var i = 0; i < relAttributes.length; i++) {
>-    var linkType = LinkToolbarHandler.getLinkType(relAttributes[i]);
>+  for (var i = 0; i < linkElement.relValues.length; i++) {
>+    var linkType = LinkToolbarHandler.getLinkType(linkElement.relValues[i]);
>     this.getItemForLinkType(linkType).displayLink(linkElement);
>   }
> }
>@@ -179,6 +178,8 @@
>   this.element = element;
>   
>   this.rel = LinkElementDecorator.convertRevMade(element.rel, element.rev);
>+  if (this.rel)
>+    this.relValues = this.rel.split(" ");
>   this.rev = element.rev;
>   this.title = element.title;
>   this.href = element.href;
>@@ -191,27 +192,14 @@
> function()
> {
>   if (!this.rel) return true;
>-  if (this.isStylesheet()) return true;
>-  if (this.isIcon()) return true;
>-
>-  return false;
>-}
>-
>-LinkElementDecorator.prototype.isStylesheet =
>-function() {
>-  if (LinkElementDecorator.findWord("stylesheet", this.rel.toLowerCase()))
>-    return true;
>-  else
>-    return false;
>-}
>-
>-LinkElementDecorator.prototype.isIcon =
>-function() {
>-  if (LinkElementDecorator.findWord("icon", this.rel.toLowerCase()))
>-    return true;
>-  else
>-    return false;
>-
>+  for (var i = 0; i < this.relValues.length; i++) {
>+    testVal = this.relValues[i].toLowerCase();
>+    if ((testVal == "stylesheet") || 
>+      (testVal == "icon") ||
>+      (testVal.match(/^p3pv/)) ||
>+      (testVal.match(/^schema./)))
>+      return true;
>+  }
>   return false;
> }
>

Comment 6

[Christopher Hoess (gone)]

Attachment: The "edit attachment as comment" UI has tricked me. how I hate it.

Comment 7

[Henrik Gemal]

Has the patch been tested with strict JavaScript warning on (Edit -> Pref ->
Debug). The "testVal" seem supisious...
Please please please test it with strict warns on.
We dont want to spend time again fixing the warnings.

Comment 8

[Christopher Hoess (gone)]

I see no warnings with the latest patch (64986) on your site, w3.org, or
bugzilla (strict warnings enabled).  Steps to reproduce?

Comment 9

[Henrik Gemal]

Just looking at the source. The variable "testVal" is missing a var.
    testVal = this.relValues[i].toLowerCase();
should be:
    var testVal = this.relValues[i].toLowerCase();

shouldn't it?

Comment 10

[Christopher Hoess (gone)]

Attachment: Added "var"

Good call, Henrik!

Comment 11

[Henrik Gemal]

With the patch I still see this:

Warning: reference to undefined property this.getKnownLinks()[linkElement.href]
Source File: chrome://navigator/content/linkToolbarItem.js
Line: 157

Do not checkin until these warnings have been fixed.

Comment 12

[Henrik Gemal]

the last patch cannot have "review". It's incomplete!
How did you test this for strict warns?

Comment 13

[Christopher Hoess (gone)]

The patch was tested by browsing a number of sites and traversing them via the
linktoolbar.  I was able to reproduce your strict warnings by turning off the
toolbar on a site with links and then turning them on again.

I was *also* able to reproduce them *with this patch backed out*.  cc'ing
sballard, who knows more about the code than I do, currently.

Please be a little more careful in assigning blame.

Comment 14

[Christopher Hoess (gone)]

Comment on attachment 64986 [details] [diff] [review]
The "edit attachment as comment" UI has tricked me.  how I hate it.

Note that r=timeless (IRC).

I'm sorry I got snippy, it's late and I'm tired.

Comment 15

[Christopher Hoess (gone)]

Comment on attachment 64986 [details] [diff] [review]
The "edit attachment as comment" UI has tricked me.  how I hate it.

Note that r=timeless (IRC).

I'm sorry I got snippy, it's late and I'm tired.

Comment 16

[Christopher Hoess (gone)]

Comment on attachment 64986 [details] [diff] [review]
The "edit attachment as comment" UI has tricked me.  how I hate it.

Note that r=timeless (IRC).

I'm sorry I got snippy, it's late and I'm tired.

Comment 17

[Stuart Ballard]

Actually the only code I really understand so far is linkToolbarOverlay.{xul,js}
- the stuff in linkToolbar{Handler,Item}.js I've mostly left alone as it's
mostly over my head. Perhaps when I get done with the xblification -- if I ever
get enough time to work on that -- I'll understand this area better.

Anyone remember who wrote this part?

Comment 18

[David Hyatt]

Comment on attachment 64989 [details] [diff] [review]
Added "var"

sr=hyatt

Comment 19

[Henrik Gemal]

is this fixed or checked in?

Comment 20

[Christopher Hoess (gone)]

Fixed.  (Forgot to mark it when I spun off the JS bug.)

Comment 21

[Henrik Gemal]

v 20020213