Closed Bug 837101 Opened 7 years ago Closed 7 years ago

Include Google search snippet on SeaMonkey website

Categories

(SeaMonkey :: Project Organization, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mcsmurf, Assigned: mcsmurf)

Details

Attachments

(3 files)

Per SeaMonkey council decision we should include a Google search snippet on the www.seamonkey-project.org/start page (default SeaMonkey start page). I'll provide the required HTML code.
FYI the standard google search snippet makes the w3c validator complain.
You mean the one with the "<gcse:searchbox-only></gcse:searchbox-only>" code? There are two variants, an older one and a newer one. The newer one replaces the <gcse:searchbox-only></gcse:searchbox-only> "HTML code" with some other tags via JS on pageload.
Jens, I think I might need your help with this later. CCing you now just to make you aware that this page will change :)
Attached patch PatchSplinter Review
Jens, what do you think of this? Maybe the <span ...> tag should be moved inside the <div>, I think it looks a bit better. Note that I'm not a web designer at all, so bear with me if I've made any stupid CSS mistake :). I've tested the changes locally so I can see the final layout.
Attachment #709661 - Flags: feedback?(jh)
Attached patch Another variantSplinter Review
I think I like that one better.
I haven't checked any of this locally yet (am pretty swamped this close to my holiday) but variant 1 is a no-go because the main heading (first h1) must not be Google - such things influence search engine results (like, you guessed it: those from Google!). Even having "Google" as the first h2 is something I have to think about (and see).

The JS needs some cleanup/simplification, but I'll do that during actual review (later today I hope).
Please note that we're not allowed to change the code, except maybe removing line breaks from the code.
Comment on attachment 709661 [details] [diff] [review]
Patch

As I said, this has the wrong heading first. I'll provide review comments for your other patch and attach a suggestion of what I'd change in a minute.
Attachment #709661 - Flags: feedback?(jh) → feedback-
Comment on attachment 709886 [details] [diff] [review]
Another variant

Review of attachment 709886 [details] [diff] [review]:
-----------------------------------------------------------------

I think the styling is good, and fortunately the blue submit button matches our theme. :-)

Code-wise I'd apply some cleanup as explained below.

::: seamonkeyproject-org/src/start/index.en.html
@@ +12,4 @@
>  
>  <script type="text/javascript" src="/download.js"></script>
>  
> +<script>

Missing type attribute in script tag, and missing HTML comment to hide JS (cf. the rest of the file).

@@ +14,5 @@
>  
> +<script>
> +  (function() {
> +    var cx = 'partner-pub-1459447916058139:5120269768';
> +    var gcse = document.createElement('script'); gcse.type = 'text/javascript'; gcse.async = true;

One line, one command.

@@ +15,5 @@
> +<script>
> +  (function() {
> +    var cx = 'partner-pub-1459447916058139:5120269768';
> +    var gcse = document.createElement('script'); gcse.type = 'text/javascript'; gcse.async = true;
> +    gcse.src = (document.location.protocol == 'https:' ? 'https:' : 'http:') +

We don't support https on smpo so no need for this check.

@@ +17,5 @@
> +    var cx = 'partner-pub-1459447916058139:5120269768';
> +    var gcse = document.createElement('script'); gcse.type = 'text/javascript'; gcse.async = true;
> +    gcse.src = (document.location.protocol == 'https:' ? 'https:' : 'http:') +
> +        '//www.google.com/cse/cse.js?cx=' + cx;
> +    var s = document.getElementsByTagName('script')[0]; s.parentNode.insertBefore(gcse, s);

Again one line, one command.

@@ +25,5 @@
>  <title>Welcome to SeaMonkey</title>
>  </head>
>  <body>
>  <h1>Welcome to SeaMonkey</h1>
> +<h2>Google</h2>

I think we can/should do without this.

@@ +27,5 @@
>  <body>
>  <h1>Welcome to SeaMonkey</h1>
> +<h2>Google</h2>
> +<div class="key-point">
> +  <!-- Place this tag where you want the search box to render -->

No point in keeping this comment.

@@ +29,5 @@
> +<h2>Google</h2>
> +<div class="key-point">
> +  <!-- Place this tag where you want the search box to render -->
> +  <gcse:searchbox-only></gcse:searchbox-only>
> +  <span class="note" style="font-size: small;">For your convenience we've included a Google search box here, it will search the whole web</span>

Missing full stop at the end, and I'd replace the comma by another full stop (plus captialized "It" of course).
Attachment #709886 - Flags: review-
Comment on attachment 710198 [details] [diff] [review]
proposed cleanup

I'm ok with one line, one command. What I'm not ok with is removing the https: check (yes, I'm serious on the don't change the code thing :) let's just do it as they say..). I'm ok with the other changes, after all there's a transparent Google overlay/logo inside the text box, so it should be clear that is is a Google search.
Can you check that change in?
Checking in src/start/index-test.en.html;
/www/seamonkeyproject-org/src/start/index-test.en.html,v  <--  index-test.en.html
new revision: 1.16; previous revision: 1.15
done
Checking in src/start/index.de.html;
/www/seamonkeyproject-org/src/start/index.de.html,v  <--  index.de.html
new revision: 1.24; previous revision: 1.23
done
Checking in src/start/index.en.html;
/www/seamonkeyproject-org/src/start/index.en.html,v  <--  index.en.html
new revision: 1.36; previous revision: 1.35
done
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Assignee: nobody → bugzilla
You need to log in before you can comment on or make changes to this bug.