Last Comment Bug 538419 - remove nsAboutAbout.js
: remove nsAboutAbout.js
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1b1
Assigned To: Justin Wood (:Callek)
:
Mentors:
about:about
Depends on: 220253 363491
Blocks:
  Show dependency treegraph
 
Reported: 2010-01-07 10:44 PST by Steffen Wilberg
Modified: 2010-08-26 03:14 PDT (History)
4 users (show)
neil: wanted‑seamonkey2.1-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Remove aboutAbout from suite/ (4.91 KB, patch)
2010-08-09 20:16 PDT, Justin Wood (:Callek)
neil: review-
Details | Diff | Splinter Review
address review comments. (10.09 KB, patch)
2010-08-24 20:09 PDT, Justin Wood (:Callek)
neil: review+
Details | Diff | Splinter Review

Description Steffen Wilberg 2010-01-07 10:44:56 PST
In bug 220253, I readded the redirector to about:about to chrome://global/content/aboutAbout.xhtml, which is now part of Toolkit.

So you may want to remove your custom redirector in nsAboutAbout.js, and either drop your own aboutAbout.html, or replace the new Toolkit version with yours.
Comment 1 Steffen Wilberg 2010-01-09 01:28:10 PST
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20100108 SeaMonkey/2.1a1pre
still displays the old about:about page.

But my version looks much better: chrome://global/content/aboutAbout.xhtml
Comment 2 neil@parkwaycc.co.uk 2010-01-13 03:33:48 PST
Don't see how this can be wanted-seamonkey2.1 when we don't know whether we'll be using 1.9.3 yet.
Comment 3 Justin Wood (:Callek) 2010-08-09 20:16:56 PDT
Created attachment 464279 [details] [diff] [review]
Remove aboutAbout from suite/

...and inherently use toolkit's version
Comment 4 neil@parkwaycc.co.uk 2010-08-10 02:21:36 PDT
Doesn't it need to be a removed file?

Should we port the CSS changes too?
Comment 5 neil@parkwaycc.co.uk 2010-08-10 02:24:58 PDT
Oh, and we need to hide about:certerror too. See bug 538421.
Comment 6 Justin Wood (:Callek) 2010-08-10 15:41:40 PDT
(In reply to comment #4)
> Doesn't it need to be a removed file?

Yes.

> Should we port the CSS changes too?

Which CSS Change specifically?

(In reply to comment #5)
> Oh, and we need to hide about:certerror too. See bug 538421.

Hiding more pages from this that *we* expose should be done, yes. But can we do it in a followup?
Comment 7 neil@parkwaycc.co.uk 2010-08-10 16:17:42 PDT
(In reply to comment #6)
> (In reply to comment #4)
> > Should we port the CSS changes too?
> Which CSS Change specifically?
Actually I only remember one.

> (In reply to comment #5)
> > Oh, and we need to hide about:certerror too. See bug 538421.
> Hiding more pages from this that *we* expose should be done, yes. But can we do
> it in a followup?
Just saying, it would look odd that neterror is hidden but certerror is not.
Comment 8 Steffen Wilberg 2010-08-11 00:07:04 PDT
The CSS change from bug 220253 was the two-column-style:
.columns {
  -moz-column-width: 20em;
  -moz-column-gap: 3em;
}

Did you consider unforking aboutAbout.xhtml and use the toolkit version?
That also takes care of hiding some about pages which are not useful without query parameters using nsIAboutModule.HIDE_FROM_ABOUTABOUT (bug 538421):
chrome://global/content/aboutAbout.xhtml
Comment 9 neil@parkwaycc.co.uk 2010-08-11 04:15:12 PDT
(In reply to comment #8)
> The CSS change from bug 220253 was the two-column-style:
> .columns {
>   -moz-column-width: 20em;
>   -moz-column-gap: 3em;
> }
Thanks.

> Did you consider unforking aboutAbout.xhtml and use the toolkit version?
This is effectively what this patch does!

> That also takes care of hiding some about pages which are not useful without
> query parameters using nsIAboutModule.HIDE_FROM_ABOUTABOUT (bug 538421):
See comment #5: some of our about pages don't have that flag when they should.
Comment 10 Steffen Wilberg 2010-08-11 06:03:32 PDT
Ah, I was confused by aboutAbout.html not being removed as well, but of course without aboutAbout.js, the toolkit version is called instead.

Yeah, you'd have to add the HIDE_FROM_ABOUTABOUT flag to
/suite/common/src/nsAboutCertError.js
and maybe /suite/feeds/src/nsAboutFeeds.js.

(Search for ALLOW_SCRIPT in /suite/:
http://mxr.mozilla.org/comm-central/search?string=ALLOW_SCRIPT&case=1&find=%2Fsuite%2F )
Comment 11 neil@parkwaycc.co.uk 2010-08-11 06:26:46 PDT
(In reply to comment #10)
> Ah, I was confused by aboutAbout.html not being removed as well
Good catch!
Comment 12 Justin Wood (:Callek) 2010-08-24 20:09:29 PDT
Created attachment 468913 [details] [diff] [review]
address review comments.

This should address all review comments.
Comment 13 neil@parkwaycc.co.uk 2010-08-25 04:24:50 PDT
Comment on attachment 468913 [details] [diff] [review]
address review comments.

>-  lockFactory: function lockFactory(lock) {
>-    /* no-op */
>-  }
It's about time we got rid of this ;-)
Comment 14 Justin Wood (:Callek) 2010-08-25 21:38:32 PDT
http://hg.mozilla.org/comm-central/rev/314ccc712341

Note You need to log in before you can comment on or make changes to this bug.