255611 - Help item "For Internet Explorer Users" should be hidden on OS/2 and Linux

Status: VERIFIED FIXED

(SeaMonkey :: Help Documentation, defect)

Description

[Peter Weilbacher]

Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.8a3) Gecko/20040811
and
Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.7.2) Gecko/20040814 Firefox/0.9.1+

This has already happened on Unix platforms for Firefox (see bug 214102). OS/2
has never had an Internet Explorer, so I think the same menu item and help
contents should be hidden on OS/2, too.
I will try to get a patch uploaded for both aviary branch and trunk following
the abovementioned bug over the weekend.

Comment 1

[Mike Kaply [:mkaply]]

What a stupid patch that was.

Just change that line to #ifdef XP_WIN - since XP_UNIX is defined for Mac, this
wasn't showing up on Mac.

This patch only broke OS/2. 

Woohoo!

Comment 2

[Peter Weilbacher]

Attachment: Enable Help->IEUsers menu item for Win and Mac only

Yes, although I guess it is supposed to show up on MacOSX, because the IEHelp
says "Note: This information applies to Mozilla Firefox for Windows and Mac
OS."

I guess there is no way to exclude this part of the help completely, the
#ifdefs of the original patch were obviously backed out since then because of
build breaks.

This patch enables the _menu item_ for Win and Mac but does nothing to any
other files. It has the desired effect on OS/2.

Comment 3

[Peter Weilbacher]

Attachment: Fix for Seamonkey on the trunk

This is basically the same fix for Seamonkey, but in the meantime I found out
how to enable the preprocessor for the help-toc.rdf file so that the IEUsers
help does not show up at all on platforms other than Win and Mac. The file
forieusers.xhtml is still packaged, though.

Comment 4

[R.J. Keller]

Comment on attachment 156209 [details] [diff] [review]
Enable Help->IEUsers menu item for Win and Mac only

Neil, can you review this? I don't know if hixie's preprocessor supports
#if defined(XP_WIN) || defined(XP_MACOSX)

If so, then we should use that. It's more than likely that it isn't supported,
however.

Comment 5

[Mike Kaply [:mkaply]]

(In reply to comment #4)
> (From update of attachment 156209 [details] [diff] [review])
> I don't know if hixie's preprocessor supports
> #if defined(XP_WIN) || defined(XP_MACOSX)

Nope, the XUL preprocessor doesn't support that.

Should the firefox patch be updated to preprocess the help ROC rdf?

Comment 6

[[e-mail]]

Comment on attachment 156217 [details] [diff] [review]
Fix for Seamonkey on the trunk

I don't know how keen mkaply is to get this menu item removed in OS2 builds but
Internet Explorer versions were created for Solaris and HP-UX.

Comment 7

[Mike Kaply [:mkaply]]

Both those platforms define XP_UNIX, so they wouldn't have had this item anyway.

We should simply change the item to keep the old behavior and remove it on OS/2.

Comment 8

[Mike Kaply [:mkaply]]

Attachment: Simple fix

Just don't add the menu for OS/2.

Comment 9

[R.J. Keller]

BTW, help TOC cannot be preprocessed for locale reasons.

Comment 10

[sairuh (rarely reading bugmail)]

shouldn't this menu item be removed for Linux as well?

Comment 11

[Mike Kaply [:mkaply]]

(In reply to comment #10)
> shouldn't this menu item be removed for Linux as well?

It already is

Comment 12

[sairuh (rarely reading bugmail)]

(In reply to comment #11) 
> It already is

should have clarified my question: I still see this menu item with a Linux trunk
build (custom install with all components, fwiw). using 2004081608-trunk (1.8a3
seamonkey) on fedora core 2. was the fix not yet checked in on the trunk?

Comment 13

[Mike Kaply [:mkaply]]

I don't think anything has been done trunk related yet.

Comment 14

[Peter Weilbacher]

(In reply to comment #9)
> BTW, help TOC cannot be preprocessed for locale reasons.

Please explain. What is the problem with locales? Can the translators not add
the #ifdefs there?

Comment 15

[Peter Weilbacher]

Attachment: Simple fix for Seamonkey

The simple fix obviously has better chances to make it into the tree...

Comment 16

[R.J. Keller]

(In reply to comment #14)
> (In reply to comment #9)
> > BTW, help TOC cannot be preprocessed for locale reasons.
> 
> Please explain. What is the problem with locales? Can the translators not add
> the #ifdefs there?

Localizers translate based on the contents of the locale directories in the JAR
files. If they are inconsistent, they'll be forced to distribute 3 JARs, which
would be unreasonable. We have workaround for the HTML (class="win", etc.) but
nothing yet for TOC.

Comment 17

[R.J. Keller]

Attachment: Patch - no duplicate code.

Comment 18

[Peter Weilbacher]

R.J., clever idea to use the extra variable. But you need to move that #endif 

+#ifdef DISPLAY_IE_USERS_HELP_MENU_ITEM
     <menuitem label="&helpForIEUsers.label;"
       accesskey="&helpForIEUsers.accesskey;"
       position="2"
       oncommand="openHelp('ieusers');" />
   </menupopup>
+#endif

up by one line.

Comment 19

[[e-mail]]

Comment on attachment 156340 [details] [diff] [review]
Patch - no duplicate code.

The help text supports IE users on Windows, Mac, Solaris and HP-UX so the menu
item should too.

Comment 20

[Mike Kaply [:mkaply]]

I'm just going to add an #ifndef XP_OS2 inside that block for now if that's OK.

Comment 21

[Mike Kaply [:mkaply]]

I checked in a Firefox fix.

This still needs some trunk lovin'.

We should probably just use the same think as Firefox  on trunk.

Comment 22

[R.J. Keller]

--> me

Hoping to get this one in soon.

Comment 23

[R.J. Keller]

Attachment: Patch - w/ solaris, OS2, and HP-UX display added

Comment 24

[Mike Kaply [:mkaply]]

Actually OS/2 doesn't want the item.

Comment 25

[R.J. Keller]

Attachment: Patch - real version

Had some confusion and got it cleared up on IRC. This should be the real one.

Comment 26

[Peter Weilbacher]

Nothing happened here in a long time, but the patch is still fine for the
current trunk and not too large for a quick review. Any chance to get this in
soon, or do you need to find someone to build on Solaris and HP-UX first?

Comment 27

[Peter Weilbacher]

Attachment: Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

I would still like to see this get into SeaMonkey, always having to build this into my unofficial OS/2 releases is getting annoying...

Comment 28

[Lewis Rosenthal]

(In reply to comment #27)
> I would still like to see this get into SeaMonkey, always having to build this
> into my unofficial OS/2 releases is getting annoying...
> 
I agree, Peter. For the UI, it creates unnecessary clutter in the Help menu. Frankly, I'd be happy with M$ putting a help menu item in IE: "Help for Mozilla Users." That would really be helpful for those times when I need to do support on Windows machines running IE...at least, before I install SeaMonkey on them. ;-)

Lewis

Comment 29

[Lewis Rosenthal]

Quick userChrome.css hack to at least hide the blasted thing:

/* Hide annoying IE help item */
menuitem[label="For Internet Explorer Users"] {
  display: none !important;
  }

Lewis

Comment 30

[Peter Weilbacher]

Thanks for the reviews. Could someone please get this into the trunk? And which flag do I have to set to ask for approval for SeaMonkey 1.0x?

Comment 31

[Peter Weilbacher]

Comment on attachment 201927 [details] [diff] [review]
Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

Checkin reminder, please: anyone?

Would also be great to see this in SeaMonkey 1.0 or at least 1.1.

Comment 32

[Robert Kaiser]

Comment on attachment 201927 [details] [diff] [review]
Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

a=mw for 1.1, first a= from me for 1.0 (second one still needed) given that it's been tested on all tier-1 platforms on trunk and/or 1.1 before it goes into 1.0

Comment 33

[Ian Neal]

Comment on attachment 201927 [details] [diff] [review]
Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

a=me second needed one from SM1.0

Comment 34

[R.J. Keller]

Fix checked in on the trunk (which I assume is Seamonkey1.1). I have no clue what branch Seamonkey 1.0 is on so if someone tells me, I'll check it into there too.

Comment 35

[Peter Weilbacher]

Thanks R.J. Trunk is correct for now, SeaMonkey 1.0 would be MOZILLA_1_8_0_BRANCH and SeaMonkey 1.1 MOZILLA_1_8_BRANCH, but give me some time to test on the trunk.

Comment 36

[R.J. Keller]

oh ok, I assumed Seamonkey 1.1 was the trunk :). I've ran this code previously with no problem so I doubt there'd be an issue.

Tell me when and I'll check it into those branches.

Comment 37

[Peter Weilbacher]

R.J., because I have some problems with sending email I am not sure if the one I sent to you arrived, so I repeat it here. I think you can check in to both these branches now. I tested with Windows and Linux and didn't see any complaints about MacOSX.

Comment 38

[Robert Kaiser]

Comment on attachment 201927 [details] [diff] [review]
Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

forwarding a+ from 1.0, this is nice very-low-risk polish.

Comment 39

[Ian Neal]

Comment on attachment 201927 [details] [diff] [review]
Update for bitrot (Checked in trunk, branch 1.8 & 1.8.0)

Checking in (branch 1.8)
jar.mn;
new revision: 1.66.8.1; previous revision: 1.66
content/helpMenuOverlay.xul;
new revision: 1.14.4.1; previous revision: 1.14
done
Checking in (branch 1.8.0)
jar.mn;
new revision: 1.66.16.1; previous revision: 1.66
content/helpMenuOverlay.xul;
new revision: 1.14.12.1; previous revision: 1.14
done

Comment 40

[Peter Weilbacher]

Thanks everyone!