Closed Bug 1588871 Opened 2 years ago Closed 2 years ago

Clean up the helpviewer (preprocessing and comments)

Categories

(SeaMonkey :: Help Viewer, enhancement)

All
macOS
enhancement
Not set
normal

Tracking

(seamonkey2.49esr wontfix, seamonkey2.63 wontfix, seamonkey2.53 fixed, seamonkey2.57esr fixed)

RESOLVED FIXED
seamonkey2.69
Tracking Status
seamonkey2.49esr --- wontfix
seamonkey2.63 --- wontfix
seamonkey2.53 --- fixed
seamonkey2.57esr --- fixed

People

(Reporter: frg, Assigned: frg)

Details

(Whiteboard: SM2.53.1)

Attachments

(1 file, 1 obsolete file)

The help viewer uses a really old code style. Clean it up somewhat before we start replacing the RDF usage in it.

Attached patch 1588871-helpviewerpre.patch (obsolete) — Splinter Review

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: nothing visible
Testing completed (on m-c, etc.): 2.53
Risk to taking this patch (and alternatives if risky): --
String changes made by this patch: some rdf help changes. No impact on l10n.

Attachment #9101360 - Flags: review?(iann_bugzilla)
Attachment #9101360 - Flags: approval-comm-release?
Attachment #9101360 - Flags: approval-comm-esr60?
Comment on attachment 9101360 [details] [diff] [review]
1588871-helpviewerpre.patch

>+++ b/suite/components/helpviewer/content/help.js

>-# toggleZLevel - Toggles whether or not the window will always appear on top. Because
>-#   alwaysRaised is not supported on an OS other than Windows, this code will not
>-#   appear in those builds.

>+// toggleZLevel - Toggles whether or not the window will always appear on top. Because
>+//   alwaysRaised is not supported on Linux, this code will not
>+//   appear in those builds.

So previously it only worked on Windows, does it now work on macOS too?

>+++ b/suite/components/helpviewer/content/platformClasses.css

>-%ifdef XP_WIN
>-.noWin, .mac, .unix { display: none; } 
>-%else
>-%ifdef XP_MACOSX
>-.noMac, .win, .unix { display: none; }
>-%else
>-.noUnix, .win, .mac { display: none; }
>-%endif
>-%endif

This hides help not applicable to the relevant platform e.g. https://dxr.mozilla.org/comm-esr60/source/suite/locales/en-US/chrome/common/help/cs_nav_prefs_navigator.xhtml#93

Are you planning to do this in a different way?

Other changes seem to be okay so f+ for the moment
Flags: needinfo?(frgrahl)
Attachment #9101360 - Flags: review?(iann_bugzilla)
Attachment #9101360 - Flags: feedback+
Attachment #9101360 - Flags: approval-comm-release?
Attachment #9101360 - Flags: approval-comm-esr60?
Comment on attachment 9101360 [details] [diff] [review]
1588871-helpviewerpre.patch

>+++ b/suite/components/helpviewer/content/contextHelp.js

>+XPCOMUtils.defineLazyModuleGetter(this, "AppConstants",
>+                                  "resource://gre/modules/AppConstants.jsm");

I also get an error in the console about XPCOMUtils not being defined so probably need:
ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
Comment on attachment 9101360 [details] [diff] [review]
1588871-helpviewerpre.patch

>+++ b/suite/locales/en-US/chrome/common/help/helpFileLayout.css
>@@ -1,12 +1,11 @@
> /* This Source Code Form is subject to the terms of the Mozilla Public
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>-@import url("chrome://help/content/platformClasses.css");
As we're keeping platformClasses.css, this change needs to be dropped too.

Previous issues hopefully addressed.

What is left in the patch is removal of preprocessing the js files, cleaning up comments and getting rid of the last os2 references. Functionality should be unchanged for all platforms.

Attachment #9101360 - Attachment is obsolete: true
Flags: needinfo?(frgrahl)
Attachment #9104540 - Flags: review?(iann_bugzilla)
Attachment #9104540 - Flags: approval-comm-release?
Attachment #9104540 - Flags: approval-comm-esr60?
Comment on attachment 9104540 [details] [diff] [review]
1588871-helpviewerclean-V2.patch

[Triage Comment]
LGTM r/a=me
Attachment #9104540 - Flags: review?(iann_bugzilla)
Attachment #9104540 - Flags: review+
Attachment #9104540 - Flags: approval-comm-release?
Attachment #9104540 - Flags: approval-comm-release+
Attachment #9104540 - Flags: approval-comm-esr60?
Attachment #9104540 - Flags: approval-comm-esr60+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/d2e4303926c9
Clean up comments and remove as much preprocessing in the helpviewer as possible. r=IanN

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

Pushed included some addtional "following blanks" removals in
https://hg.mozilla.org/releases/comm-esr60/rev/a034a02220f00215ce87c5662432cf9af05a9508

Target Milestone: --- → seamonkey2.69
Whiteboard: SM2.53.1
You need to log in before you can comment on or make changes to this bug.