"p3pv1" link in Site Navigation toolbar?

VERIFIED FIXED

Status

SeaMonkey
UI Design
VERIFIED FIXED
17 years ago
14 years ago

People

(Reporter: Henrik Gemal, Assigned: Eric Hodel)

Tracking

Trunk
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

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

17 years ago
->drbrain-bugzilla@segment7.net
Assignee: trudelle → drbrain-bugzilla
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.
Blocks: 103053
QA Contact: sairuh → claudius
Created attachment 64970 [details] [diff] [review]
Changed data structures a little, for convenience in solving this.
Attachment #64970 - Attachment is obsolete: true
Created attachment 64984 [details] [diff] [review]
Fixed patch per timeless
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;
> }
>
Created attachment 64986 [details] [diff] [review]
The "edit attachment as comment" UI has tricked me.  how I hate it.
Attachment #64984 - Attachment is obsolete: true
Attachment #64986 - Flags: review+
(Reporter)

Comment 7

17 years ago
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.
I see no warnings with the latest patch (64986) on your site, w3.org, or
bugzilla (strict warnings enabled).  Steps to reproduce?
(Reporter)

Comment 9

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

Created attachment 64989 [details] [diff] [review]
Added "var"

Good call, Henrik!
Attachment #64989 - Flags: review+
(Reporter)

Comment 11

17 years ago
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.
(Reporter)

Comment 12

17 years ago
the last patch cannot have "review". It's incomplete!
How did you test this for strict warns?
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.
Attachment #64986 - Attachment is obsolete: true
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 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 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

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

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

sr=hyatt
Attachment #64989 - Flags: superreview+
(Reporter)

Comment 19

17 years ago
is this fixed or checked in?
Fixed.  (Forgot to mark it when I spun off the JS bug.)
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Reporter)

Comment 21

17 years ago
v 20020213
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.