Closed Bug 396105 Opened 17 years ago Closed 16 years ago

Hide "For Internet Explorer Users" menuitem (and Help content)

Categories

(SeaMonkey :: Help Documentation, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a1

People

(Reporter: stefanh, Assigned: neil)

References

()

Details

Attachments

(5 files, 2 obsolete files)

We should hide the menuitem and the related help content for mac users. IE5 for mac hasn't been available for download from MS since Jan 31 2006. I also doubt that it works on Tiger (in case someone still has IE5 on their hard drive).
Can we move it to help-win and forget about the rest of the platforms? I guess that Solaris and HP-UX IE versions are even in worse shape than Mac one...
(In reply to comment #1)
> Can we move it to help-win and forget about the rest of the platforms? I guess
> that Solaris and HP-UX IE versions are even in worse shape than Mac one...
> 

Yeah, good idea. Hmm, I guess we could get rid of help-win and put the stuff back in the toc and use the nc:platform attribute (see bug 296012).
Sounds like a plan... Maybe I'll use this bug as a test (as long as you can test the patch first), before adventuring in fixing the rest of our plaform specific help content.
Note to self: remember to use bug 289776 as a guide.
Before hiding the help on non-Windows platforms it might be an idea to hide the menuitem that the help refers to too...
Summary: [Mac] Hide "For Internet Explorer Users" menuitem (and Help content) → Hide "For Internet Explorer Users" menuitem (and Help content)
Since I happen to have a patch for just the menuitem I suggest that we do this in 2 steps. Regarding step 2 (hide/show content): Neil, are you fine with using the nc:platform attribute (implemented in bug 296012) and getting rid of help-win.rdf?
Attachment #286292 - Flags: superreview?(neil)
Attachment #286292 - Flags: review?(neil)
OS/2 is also using chrome files in /win/, so I don't think you should do it this way.
Comment on attachment 286292 [details] [diff] [review]
Part 1 - show menuitem on windows only

(In reply to comment #7)
> OS/2 is also using chrome files in /win/, so I don't think you should do it
> this way.
>

Oops, sorry. We should keep the ifdef then.
Attachment #286292 - Attachment is obsolete: true
Attachment #286292 - Flags: superreview?(neil)
Attachment #286292 - Flags: review?(neil)
I'm moving the preprocessing to the win version of platformCommunicatorOverlay.xul. Larger patch, but cleaner than keeping an ifdef in utilityOverlay.xul imo. Only tested on mac...
Attachment #286329 - Flags: superreview?(neil)
Attachment #286329 - Flags: review?(neil)
Comment on attachment 286329 [details] [diff] [review]
New OS/2-friendly version (checked in)

Works fine on Windows.
Attachment #286329 - Flags: superreview?(neil)
Attachment #286329 - Flags: superreview+
Attachment #286329 - Flags: review?(neil)
Attachment #286329 - Flags: review+
Attachment #286329 [details] [diff] landed:

Checking in suite/common/jar.mn;
/cvsroot/mozilla/suite/common/jar.mn,v  <--  jar.mn
new revision: 1.30; previous revision: 1.29
done
Checking in suite/common/utilityOverlay.xul;
/cvsroot/mozilla/suite/common/utilityOverlay.xul,v  <--  utilityOverlay.xul
new revision: 1.54; previous revision: 1.53
done
Checking in suite/common/win/platformCommunicatorOverlay.xul;
/cvsroot/mozilla/suite/common/win/platformCommunicatorOverlay.xul,v  <--  platformCommunicatorOverlay.xul
new revision: 1.7; previous revision: 1.6
done
Checking in suite/locales/en-US/chrome/common/utilityOverlay.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/common/utilityOverlay.dtd,v  <--  utilityOverlay.dtd
new revision: 1.33; previous revision: 1.32
done
Checking in suite/locales/en-US/chrome/common/win/platformCommunicatorOverlay.dtd;
/cvsroot/mozilla/suite/locales/en-US/chrome/common/win/platformCommunicatorOverlay.dtd,v  <--  platformCommunicatorOverlay.dtd
new revision: 1.7; previous revision: 1.6
done
Attachment #286329 - Attachment description: New OS/2-friendly version → New OS/2-friendly version (checked in)
Giacomo, if you don't mind I'll fix the rest (started working on a patch). That said, there might be some perf issue if we move to using nc:platform attributes directly in suite-toc/help-index1. Not sure how much we loose, though - and I guess this should be compared with the benefits of removing help-win.rdf
Here's a complete patch that includes removal of help-win.rdf. Atm just for reference.
Stefan, please go ahead. Working around the clock for a web app and getting everything ready for the marriage leaves little to no time for working on anything else. My subtle plan is finish off the web app in advance, start working slowly on the next project while secretely prepare a few patch for SM :). We'll see how it works out.
So.. (talked to Neil) it appears that we have the perf issue already. The code in toolkit's help.js crawls the whole tree for platform attributes.
Here's an alternative version that, instead of removing help-win.rdf, adds the ie users content to help-win.rdf. The problem with this patch is that the "For Internet Explorer Users" item ends up at the bottom (after tools and development).
Attachment #286412 - Attachment description: Alternate verson → Alternate version
Neil reports that the toolkit filtering has some bad perf issues.
(In reply to comment #17)
> Neil reports that the toolkit filtering has some bad perf issues.
>

(8.4 seconds to open the help window on a p3 733MHz)

Neil and I had a discussion on irc regarding the perf hit. There might be some other way to do the platform-filtering. So, I'll hold on a bit with this until we know.
Depends on: 401968
Attached patch Proposed patch (obsolete) — Splinter Review
This version uses the fix for bug 401968 to allow the ieusers item to appear in the correct place on Windows.
Assignee: stefanh → neil
Status: NEW → ASSIGNED
Attachment #294349 - Flags: review?(stefanh)
Attached patch -w versionSplinter Review
You might find this slightly easier to review.
Comment on attachment 294349 [details] [diff] [review]
Proposed patch

Would it be possible to use the same logic to the other items in help-win.rdf (placeholders in suite-toc)?
(In reply to comment #22)
>(From update of attachment 294349 [details] [diff] [review])
>Would it be possible to use the same logic to the other items in help-win.rdf
>(placeholders in suite-toc)?
Actually with the shell service we might be moving those into suite-toc anyway.
Comment on attachment 294349 [details] [diff] [review]
Proposed patch

Since non-windows users can still find references to "Internet Explorer" and, for instance, searching for the word "Terminology" will display the "Terminology Differences" content, I think you also need to:

1) Remove the whole <rdf:Description about="#ieusers">, starting at line 899 in help-index1.rdf

2) Remove the "ieusers" subheading starting at line 815 help-index1.rdf

1) and 2) can be removed completely because the same search will display 3) for a windows user.

3) Move the <rdf:Description about="#ieusers"> in suite-toc.rdf to help-win.rdf

If you bother, please also remove <rdf:li><rdf:Description ID="appearance_pref_themes" nc:name="Themes" nc:link="cs_nav_prefs_appearance.xhtml#themes"/> </rdf:li> from suite-toc.rdf while you're at it... I must have forgotten that one :-/

Note: I still think we shall use the same logic. But of course, if there will be a shell service for all platforms, there's no need to touch the "default browser" stuff now.
(In reply to comment #24)
>1) and 2) can be removed completely because the same search will display 3) for
>a windows user.
I want to keep the index up-to-date in case we ever restore it.

>If you bother, please also remove <rdf:li><rdf:Description
>ID="appearance_pref_themes" nc:name="Themes"
>nc:link="cs_nav_prefs_appearance.xhtml#themes"/> </rdf:li> from suite-toc.rdf
>while you're at it... I must have forgotten that one :-/
It's not as simple as that, so the work will need to be done in a separate bug.
Attachment #294349 - Attachment is obsolete: true
Attachment #294570 - Flags: review?(stefanh)
Attachment #294349 - Flags: review?(stefanh)
Comment on attachment 294570 [details] [diff] [review]
Addressed review comments

<rdf:Description about="help-index1.rdf#browser">
   <nc:subheadings>
@@ -41,6 +77,41 @@
    </nc:subheadings>
 </rdf:Description>
 
+<rdf:Description about="help-indexAZ.rdf#i">
+   <nc:subheadings>
+     <rdf:Seq><rdf:li>
+       <rdf:Description ID="ieusers"
+         nc:name="Internet Explorer User Help"
+	 nc:link="forieusers.xhtml"/>
+     </rdf:li></rdf:Seq>
+   </nc:subheadings>
+</rdf:Description>
+
+<rdf:Description about="#ieusers">

Looks like this one should be about="help-index1.rdf#ieusers"
Attachment #294570 - Flags: review?(stefanh) → review+
(In reply to comment #27)
>(From update of attachment 294570 [details] [diff] [review])
>>+<rdf:Description about="help-indexAZ.rdf#i">
>>+   <nc:subheadings>
>>+     <rdf:Seq><rdf:li>
>>+       <rdf:Description ID="ieusers"
>>+         nc:name="Internet Explorer User Help"
>>+        nc:link="forieusers.xhtml"/>
>>+     </rdf:li></rdf:Seq>
>>+   </nc:subheadings>
>>+</rdf:Description>
>>+
>>+<rdf:Description about="#ieusers">
>Looks like this one should be about="help-index1.rdf#ieusers"
No; #i is help-indexAZ only because it's linking to the other RDF file.
#ieusers is linking to the #i subentry above.
Help patch checked in -> Fixed.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.