Closed Bug 119858 Opened 23 years ago Closed 23 years ago

"p3pv1" link in Site Navigation toolbar?

Categories

(SeaMonkey :: UI Design, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: drbrain-bugzilla)

References

()

Details

Attachments

(1 file, 3 obsolete files)

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
->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
Attachment #64970 - Attachment is obsolete: true
Attached patch Fixed patch per timeless (obsolete) — Splinter Review
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;
> }
>
Attachment #64986 - Flags: review+
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?
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?

Attached patch Added "var"Splinter Review
Good call, Henrik!
Attachment #64989 - Flags: review+
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.
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.
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 on attachment 64989 [details] [diff] [review]
Added "var"

sr=hyatt
Attachment #64989 - Flags: superreview+
is this fixed or checked in?
Fixed.  (Forgot to mark it when I spun off the JS bug.)
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: