135181 - Add Translate Page to the Tools menu

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[WD]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9+)
Gecko/20020403
BuildID:    2002040303

Up until now, there has been a menu option under View called Translate.  It
would take the current URL and send it to teletranslator automagically.   Very
handy for translating articles on groups.google.com etc...

This menu item is missing

Reproducible: Always
Steps to Reproduce:
1.Click on View menu
2.
3.

Actual Results:  No Translate entry there

Expected Results:  Translate is there

Comment 1

[Boris Zbarsky [:bzbarsky]]

to menu people

Comment 2

[Blake Ross]

I believe the translation service itself wants this gone...

Comment 3

[WD]

If that's the case then perhaps http://www.babelfish.altavista.com could be used
instead?

Page translation a click or two away is a very nice feature to have.

Comment 4

[Jesse Ruderman]

http://translate.google.com/translate?u=http://www.terra.es/&hl=en
http://translate.google.com/translate?u=http://www.mozilla.org/&hl=es

Comment 5

[Gervase Markham [:gerv]]

Blake:

Netscape have decided to eliminate "Translate" from their version of the browser
for commercial reasons. Based on user demand, mozilla.org have decided to keep
it. If you check, it's in the Mozilla version of the Tools menu in the spec.

Please could you bring it back? :-)

If the translation service folds, we'll change services - but until then, we
should use the one we always have, because they have cool remote XUL which we
can show off.

Gerv

Comment 6

[Gervase Markham [:gerv]]

This has now been confirmed by Pixeljockeys - and it was in the spec all along
(for Mozilla builds.) It's very useful to non-English speakers, so if you could
resurrect it, that would be great :-)

(Anyone else CCed on this bug, feel free to look through CVS/Bonsai history, get
the old UI code out and provide a patch. I don't know if Blake nuked the
back-end JS as well; that might also need restoring.)

Gerv

Comment 7

[Neil Marshall]

Should this go back in the view menu or would it be better to place it somewhere
else, such as the tools or edit menu?

Comment 8

[Alex Bishop]

> Should this go back in the view menu or would it be better to place it somewhere
> else, such as the tools or edit menu?

As Gerv said in comment 5: "If you check, it's in the Mozilla version of the
Tools menu in the spec."

Comment 9

[Neil Marshall]

Where is the spec?  I've seen it before, but I can't seem to find it.

Comment 10

[Alex Bishop]

> Where is the spec?

http://www.mozilla.org/mailnews/specs/proposals/MenuFrame.html

Comment 11

[Neil Marshall]

Attachment: Adds Translate to the view menu

The back end for translate was still there.  This patch just adds the option to
the tasks menu.  I couldn't get it in the exact same place that the spec says
it should be, but I placed it as close.

Also, shouldn't it say something along the lines of "Translate web page"? (I
just used what was already there)

Comment 12

[Gervase Markham [:gerv]]

> Adds Translate to the view menu

Presumably you mean the Tools menu :-)

> I couldn't get it in the exact same place that the spec says
> it should be, but I placed it as close.

The placing should be made so that it's always inserted above the first separator.

> Also, shouldn't it say something along the lines of "Translate web page"? (I
> just used what was already there)

Yes. "Translate Page" seems like a much better wording to me.

Gerv

Comment 13

[Neil Marshall]

Attachment: Adds the "Translate Page" option to the proper location

Comment 14

[timeless]

imo the placement of translate is awful.

Search
... Manager
... Manager
... Manager
Translate
-
... Manager
... Manager
-
Web Dev >

If we're going to make this useful, we should group things by type.

I'd suggest (and I'll ask pixel jockeys about it) that we put translate after 
search the web, and perhaps add a dictionary lookup next to it.

changes like:
-          insertbefore="navBeginGlobalItems">
+          insertbefore="menu_translate">
should indicate that what's being implemented isn't really right.

... insertbefore should probably accept a comma separated list of id's so 
that cookie manager could say insert before ~imagemanager,navBeginGlobal~

Comment 15

[Alex Bishop]

I agree with Timeless with regards to the placement of the Translate feature.
Managers should be kept together in one place (this is a philosophy I subscribe
to for human kind as well - they can do less damage that way).

Comment 16

[Gervase Markham [:gerv]]

It's a trade-off between having more-frequently-accessed features at the top,
and grouping by type. (FYI, currently, the divider is between Navigator-only
Managers and global Managers.) 

There's a counter-argument that if we break up the Manager list, it's actually
easier to find the one you want, because the list is less uniform to the eye.

Gerv

Comment 17

[greatmp3]

I would be nice to have a pref to select the translator site (teletranslator,
babelfish, etc)

Comment 18

[Gervase Markham [:gerv]]

There is one; there's just no UI for it. Use LXR to read "all.js" to find out
its name.

Gerv

Comment 19

[Max]

I agree with [e-mail] . It would be very nice if you could choose the
translation service in prefs: Google translate, babelfish or whatever. M.

Comment 20

[Neil Marshall]

is anyone going to decide on a position for it?  Change the spec to match the
patch or the patch to match the spec, just someone make a decision.  The 2
patches are bit rotting.

Comment 21

[Gervase Markham [:gerv]]

We should implement the spec. If timeless wants to change the spec, he can
arrange for that. If the current patch adds it just above the separator, then
someone should test it, review it, and Neil should drive it to checkin.

Gerv

Comment 22

[Neil Marshall]

Attachment: Updated patch

Poor Hunk #1 shouldn't be rejected anymore with this patch.  It's updated to
work with the latest trunk.

Comment 23

[Neil Marshall]

Comment on attachment 91349 [details] [diff] [review]
Updated patch

r=FrodoB

Comment 24

[Gervase Markham [:gerv]]

Neil: have you asked anyone to sr= this patch?

Gerv

Comment 25

[Neil Marshall]

IIRC I sent an email to alexf (whoever was listed as sr for the browser) but
never got a response.

Comment 26

[Blake Ross]

This was removed from Netscape (Mozilla's largest current distributor) because
it wasn't getting much usage at all.  It seems strange to me, then, that Mozilla
would want this in their base product.

Comment 27

[Gervase Markham [:gerv]]

My understanding was that it was removed because the website at which it's
pointed wouldn't pay Netscape enough money ("they couldn't come to a commercial
arrangement.") And we've had several requests to bring it back, and it's a good
demo of remote XUL as well.

Gerv

Comment 28

[Neil Marshall]

I am leaving for vacation so someone else will have to push for a sr=

Comment 29

[Brendan Eich [:brendan]]

Who is FrodoB and when did he become a peer or owner of
http://www.mozilla.org/owners.html#XPApps, to be able to r= this patch?

/be

Comment 30

[Neil Marshall]

Not sure, someone in #mozilla who looked over the patch.

Comment 31

[Alex Bishop]

FrodoB is Mark Anderson
http://www.mozillazine.org/members/profile.html?user=145

Comment 32

[Neil Marshall]

I give up then.  I can't figure out how to actually get a patch into mozilla
properly.

Comment 33

[Gervase Markham [:gerv]]

Neil: I wrote a document explaining exactly how it's done, and it's here:
http://www.mozilla.org/hacking/
It's linked from every page on mozilla.org, via the sidebar link "Hacking".
Presumably, since you've done a lot of Mozilla hacking, you've read it?

I don't know how we can make this procedure document's location more obvious.

I share your frustration with the lack of response from super-reviewers. But you
can't blame them so much if you don't do things in the right order...

Gerv

Comment 34

[Neil Marshall]

yes I've read the document and I still can't seem to get the process right. 
I've been trying for ages now and I still don't get it.

Comment 35

[Boris Zbarsky [:bzbarsky]]

This has nothing to do with super-reviewers.  It's worse.  The fact is, r= means
"module owner or peer review".  Unfortunately we have ownerless modules,
unresponsive module owners, out-of-date information (less of this now thanks to
Brendan's recent update).  At least with sr you can get any sr to do it.  With
review there're in theory a very few people allowed to do it for any given
module.  If they never get to it, you're SOL.

Comment 36

[Brendan Eich [:brendan]]

dbaron's a despot, he did most of the work cleaning up despot.mozilla.org's
database (whence http://www.mozilla.org/owners.html comes -- a tip of the hat to
[e-mail] for extending despot so that you can do a login-less query, as
the new single-text-input form on owners.html does, to find what module contains
a given file in the source tree).

In the case of this patch, we have an owned module.  I don't know if hewitt is
reading bugmail right now, but have you tried mailing him asking for r=?  He may
well agree with blake, but you'd have to ask to be sure.

/be

Comment 37

[Pavel Franc - Mozilla.cz]

So, was there any response from module owner or peer review?

I still don't see the point of having the whole backend in Mozilla and no 
option in tools menu or context menu. Moreover, I don't care about Netscape's
decidion to dump some feature, I guess this program is called Mozilla.

Comment 38

[Gervase Markham [:gerv]]

I don't think anyone really asked for it in any significant way. This patch has
become orphaned; it needs a new owner if it is to live again.

Gerv

Comment 39

[James Graham]

Attachment: Slight update of patch

I've had a play with patchmaker to get the minor change needed for this patch
to work with the current trunk (the translate page menu item is now under the
popup manager menu item).

Comment 40

[[e-mail]]

Comment on attachment 116065 [details] [diff] [review]
Slight update of patch

Wallet needs to know where to put the Form manager menu.

Quoted from the spec:

App specific items at top. Cross product items below.

So, is Translate app specific?

Comment 41

[Alex Bishop]

> So, is Translate app specific?

Yes.

The spec says that it should be:

Search the Web
Form Manager
Cookie Manager
Image Manager
Translate
----------------
Password Manager
Download Manager

Which, when you add the Popup Manager in, makes it:

Search the Web
Form Manager
Cookie Manager
Image Manager
Popup Manager
Translate
----------------
Password Manager
Download Manager

Personally, I think the Translate item should go above the managers but I don't
think this is really the place to debate the spec.

Comment 42

[James Graham]

> Wallet needs to know where to put the Form manager menu.

From the unchanged walletNavigatorOverlay.xul:

    <menu label="&walletFormManager.label;"
          accesskey="&walletFormManager.accesskey;"
          id="wallet"
          insertafter="menu_searchWeb">

Is that not right? If not can you be a little more specific, I'm new to this :)

The layout with the patch applied is as per the prototype in the spec and
repeated in comment 41
It does look a bit odd, under all the managers without a seperator, but as
mentioned in comment 18 it is unlikely to be used enough to warrant being second
on the menu with web search. But whatever gets decided...

Comment 43

[[e-mail]]

Comment on attachment 116065 [details] [diff] [review]
Slight update of patch

OK, it looks like I misread the patch last time. Sorry :-)

Comment 44

[James Graham]

Any chance of getting a superreview on this?

Comment 45

[Gervase Markham [:gerv]]

jag's very busy; I suggest you try another superreviewer.

Gerv

Comment 46

[Alec Flett]

Comment on attachment 116065 [details] [diff] [review]
Slight update of patch

sr=alecf but do NOT check this in until you have module owner approval.
(that's jag I think?)

Comment 47

[timeless]

note that neil is now moa

Comment 48

[Neil Marshall]

What is moa?

Comment 49

[James Graham]

Hmm. I gave up on this after the roadmap depreciated seamonkey. Since there's no
evidence that's going to happen anytime soon, I suppose I may as well finish
this off. 

Anyone who wants to spend a few minutes getting this patch to apply to the
current trunk (and then get whatever reviews and approvals are needed) is more
than welcome to take this off me though :)

Comment 50

[Christian :Biesinger (don't email me, ping me on IRC)]

Neil: moa means "module owner approval"; timeless meant that neil rashbrook who
gave review also gave the module owner approval alecf asked for

james: timeless checked your patch in already:
11/17/2003 12:40	timeless%mozdev.org 	mozilla/ xpfe/ browser/ resources/ locale/
en-US/ navigator.dtd 	1.168 	1/1  	Bug 135181 Add Translate Page to the Tools menu
patch by [e-mail]/[e-mail] r=neil(moa) sr=alecf
11/17/2003 12:40	timeless%mozdev.org 	mozilla/ xpfe/ browser/ resources/
content/ navigatorOverlay.xul 	1.286 	1/0  

Comment 51

[James Graham]

Yeah, I just saw that too. Thanks timeless

Comment 52

[Jo Hermans]

I was the one that filed bug 135181, to remove the teletranslator.com
preferences. I guess we can file that bug as INVALID now. But shouldn't we
replace teletranslator.com with another service (Babelfish, Google, ...) ? Their
website doesn't exists anymore.

Comment 53

[Jo Hermans]

Eh ... bug 195783 ofcourse :-)

Comment 54

[James Graham]

The prefs that control the translation service seem to be
browser.translation.service and browser.translation.serviceDomain. I guess we
just need to patch all.js to default to something else?

Comment 55

[James Graham]

Incidentally, I ran with this patch back in March and the translation was
working then. I also seem to remember some extension that restored this
functionality, but I can't find it at the moment.

Comment 56

[James Graham]

Yeah OK, teletranslator is dead. I think google might do the job though; it's
URL format seems compatible with the rather simple Mozilla Translate() function.

Comment 57

[James Graham]

Attachment: Change default translation service

II think this is what's required to have google translate as the default
translation service. I don't know if that's the right thing to do though.

Comment 58

[Stefan Borggraefe]

I just did a fresh build. The "Translate page"  menu item is between the Form
Manager and the Cookie Manager and looks misplaced there. IMHO it should be
above all managers right below "Search the Web".

Comment 59

[James Graham]

Re: Comment #58 AFAICT, that's because timeless didn't check in the changes to
extensions/cookie/resources/content/cookieNavigatorOverlay.xul Is there any
reeason for this? I think they apply cleanly...

Oh and I think the diff on my last patch is wrong (in the sense that it hasn't
been taken relative to the CVSROOT). But I'll deal with that tomorrow.

Comment 60

[Alex Bishop]

Comment on attachment 135760 [details] [diff] [review]
Change default translation service

>+pref("browser.translation.service", "http://translate.google.com/translate?u=");

That works but to get the 'Back to Language Tools' link to work you need to
make it:

http://translate.google.com/translate?prev=/language_tools&u=

From my limited testing, it seems to guess the page language okay but it always
translates into English. This can be fixed by adding a hl parameter to the URL:

http://translate.google.com/translate?hl=fr&prev=/language_tools&u=

Localisers could tweak the pref to fit their localisation.

AltaVista's BabelFish does more languages but its URL format requires the
presence of both a source and target language:

http://babelfish.altavista.com/babelfish/tr?lp=fr_en&tt=url&url=

Without the lp parameter, it goes to
http://babelfish.altavista.com/babelfish/tr with the URL prefilled in the form
(not exactly seamless but not terrible either).

Comment 61

[James Graham]

Ignore the part in comment 59 about cookieNavigatorOverlay.xul changes applying
cleanly; I'm probably talking total rubbish. Sorry for all the bugspam :(

Comment 62

[timeless]

Attachment: changes

Note that the other change violated the spec, so I discarded it when I
commited.

Comment 63

[Johnny Stenback (:jst)]

Comment on attachment 135876 [details] [diff] [review]
changes

sr=jst

Comment 64

[Johnny Stenback (:jst)]

Comment on attachment 135876 [details] [diff] [review]
changes

sr=jst

Comment 65

[timeless]

.

Comment 66

[timeless]

i think this is fixed

Comment 67

[Alex Hosking]

Is this fixed in firebird? I don't see a translate button.

Comment 68

[James Graham]

This bug is seamonkey only. If you want the feature in firebird, open a new bug.
There's a reasonable chance it will be wontfix'd though - you might be better
off writing a tiny extension.