575182 - Cannot change Character Encoding from Firefox Menu Button

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The Firefox Menu Button doesn't have Character Encoding menu, so, we cannot change the page's encoding to right one when Fx failed to detect the right one.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

FYI: There is bug 63054, so, if we don't want to add them to the Firefox Menu Button, now might be good chance designing new UI.

Comment 2

[Maniac Vlad Florin (:vladmaniac)]

Reproduces on Mozilla/5.0 (Windows; Windows NT 5.1; en-US; rv:2.0b2pre) Gecko/20100711 Minefield/4.0b2pre 

Same problem when changing encoding of Page Source

Comment 3

[Benjamin Smedberg]

Blocking for at least a decision/resolution. I'm pretty sure we don't want the charset selector in the firefox menu, but I don't know what the solution is for when we get it wrong.

Comment 4

[Jo Hermans]

Note that the old menubar is still available if you press F10 or Alt. Unfortunately, not many people seem to know that.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Limi/Faaborg: can we get a decision as asked for in comment 3? Do we want to selectively show the character encoding menuItem for locales which need it more than others (which tends to be the Asian ones, from what I can tell)

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

Default placement is in the developer menu, locals can opt to move it to the overflow extension area so that it is directly in the main menu:

http://people.mozilla.com/~faaborg/files/20100718-firefoxButtonDetails/firefoxButton-i4.png

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

That sounds reasonable - is that where it is in beta3? I'm happy to move this to a b4 blocker that depends on some bug implementing the changes to the Firefox menu, but don't want to let this slip out of view.

Comment 8

[Alex Faaborg [:faaborg] (Firefox UX)]

>That sounds reasonable - is that where it is in beta3?

As far as I can tell the Firefox menu isn't actively being worked on at the moment.  If the freeze is monday, we are looking at beta 4.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Not sure that Limi is the best assignee for this bug.

Comment 10

[Alex Faaborg [:faaborg] (Firefox UX)]

I'm would like to dupe this change forward to bug 583386 so that we can centralize discussion, work, and blocking status for beta 4.

I'm keeping this bug separate for now since it already has blocking status. When bug 583386 picks up blocking, we might just do all of the changes over there.

Comment 11

[Shawn Wilsher :sdwilsh]

(In reply to comment #10)
> I'm would like to dupe this change forward to bug 583386 so that we can
> centralize discussion, work, and blocking status for beta 4.
> 
> I'm keeping this bug separate for now since it already has blocking status.
> When bug 583386 picks up blocking, we might just do all of the changes over
> there.
It has blocking, so duping forward.

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Bug 583386 was fixed but there is still no character encoding menu in Firefox button.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100821 Minefield/4.0b5pre

Comment 13

[u88484]

(In reply to comment #12)
> Bug 583386 was fixed but there is still no character encoding menu in Firefox
> button.
> 
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100821
> Minefield/4.0b5pre

That was because the coder couldn't figure out how the code worked for this.  I tried to help him and also could not figure it out.

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If a page doesn't tell its encoding by HTTP header or <meta>, we need to guess the right encoding by the binary pattern. Of course, that must be sometimes wrong. Then, users need to change the encoding manually.

And also, if a page tell wrong encoding, then, users need to change the encoding manually too.

Such pages are not major in current Web. However, it's not zero. So, if mismatch of encoding happens (that is called Mojibake in Japan, the term is very popular including light users) and there is no UI for changing the encoding, that makes serious a11y problem.

Comment 15

[Alex Limi (:limi) — Firefox UX Team]

(In reply to comment #14)
> Such pages are not major in current Web. However, it's not zero. So, if
> mismatch of encoding happens (that is called Mojibake in Japan, the term is
> very popular including light users) 

Yes, there are specific locales where this problem is more pronounced than others. Those locales could have the character menu as a top-level option.

Comment 16

[Alex Faaborg [:faaborg] (Firefox UX)]

>and there is no UI for changing the
>encoding, that makes serious a11y problem.

there will be a UI, this bug is blocking+

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #15)
> (In reply to comment #14)
> > Such pages are not major in current Web. However, it's not zero. So, if
> > mismatch of encoding happens (that is called Mojibake in Japan, the term is
> > very popular including light users) 
> 
> Yes, there are specific locales where this problem is more pronounced than
> others. Those locales could have the character menu as a top-level option.

So, do you mean that the character encoding menu should be displayed only on some locales? If so, we should have a new hidden pref for displaying them forcibly. Nightly builds, some Alphas, and early Betas are not localized. So, testers and developers in such locales need to display the menus on En-US build too.

Comment 18

[Alex Faaborg [:faaborg] (Firefox UX)]

Every local will have access to character encoding through the developer menu.  We would like to introduce a pref for locales to opt in to additionally displaying it directly in the main menu itself as well.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

This bug is currently assigned to Limi, but I'm pretty sure he's not going to be introducing this preference. Who's doing that work?

Comment 20

[Alex Faaborg [:faaborg] (Firefox UX)]

>Who's doing that work?

Johnath: this bug could use an owner

Comment 21

[Michael Lefevre]

Given that this is an open beta 4 blocker, and it obviously missed beta 4, shouldn't it now be moved to being a beta 6 blocker? (It's not showing up in the blocking queries at the moment).

Comment 22

[Mike Beltzner [:beltzner, not reading bugmail]]

Yes. I'm also just going to assign it so someone, eenie meenie miney Margaret!

Comment 23

[:Margaret Leibovic]

Okay. Do I just need to add the character encoding menu to the developer menu? Or should I also introduce the pref to display it directly in the main menu, and if so, where in the main menu should it go?

Comment 24

[:Margaret Leibovic]

Attachment: patch

I added the "character encoding" menu at the bottom of the developer menu, below "work offline." Is that where we want it?

Also, I implemented the change by copying the xul from browser/base/content/browser-menubar.inc, but I noticed that there's an overlay in toolkit (toolkit/content/charsetOverlay.xul), so should I try using that instead?

Comment 25

[u88484]

(In reply to comment #24)
> Created attachment 471192 [details] [diff] [review]
> patch
> 
> I added the "character encoding" menu at the bottom of the developer menu,
> below "work offline." Is that where we want it?

Here is a link to iteration 6 of the Firefox button where Alex has it like:

-------------
View Page Source
-------------
Character Encoding
-------------
Work Offline

Comment 26

[:Margaret Leibovic]

Attachment: patch v2

I corrected the ordering of the menu, and I created a browser-charsetmenu.inc file to factor out the character encoding menu code.

Comment 27

[Alex Faaborg [:faaborg] (Firefox UX)]

Comment on attachment 471192 [details] [diff] [review]
patch

Let's make work offline last, so place this above work offline and below veiw source.  With that change ui-r+

Comment 28

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 471275 [details] [diff] [review]
patch v2

>diff --git a/browser/base/content/browser-charsetmenu.inc b/browser/base/content/browser-charsetmenu.inc

>+# The Initial Developer of the Original Code is
>+# Mozilla Corporation.

This should be Netscape Communications Corporation.

>\ No newline at end of file

Fix that. Also this file has been infected with tabs, fix that too :)

>diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul

>               <menuitem id="appmenu_pageSource"
>                         label="&viewPageSourceCmd.label;"
>                         command="View:PageSource"/>
>+			  <menuseparator/>

This also got tabbed.

r=me with that fixed.

Comment 29

[:Margaret Leibovic]

Attachment: final patch

Fixed license block and tabs.

Comment 30

[Dão Gottwald [:dao]]

patching file browser/base/content/browser.xul
Hunk #1 FAILED at 566

Comment 31

[:Margaret Leibovic]

Attachment: final patch (should apply now)

This should apply after the patch for bug 589139 lands.

Comment 32

[:Margaret Leibovic]

http://hg.mozilla.org/mozilla-central/rev/7da00dcb512e

Comment 33

[Vlado Valastiak [:wladow]]

How are we going to deal with accesskeys? They should be visible in the main menu, but we don't use them in Firefox menu.

Comment 34

[Alex Faaborg [:faaborg] (Firefox UX)]

alt = normal menu
alt-f = focused firefox menu (planned), followed by arrow key access to the sub-menu.

Comment 35

[Vlado Valastiak [:wladow]]

Maybe I wasn't clear enough - I meant we should display accesskeys in the main menu (on particular platforms), but we shouldn't display them in the Firefox menu. As the code for populating the Character encoding submenu is shared, we are currently displaying them in both main and Firefox menu.
That means as of now the Character encoding is the only submenu in the new Firefox Menu having the accesskey displayed. And that's pretty inconsistent - either every Firefox menu item should have an accesskey or there should be no accesskeys at all.

Comment 36

[:Gavin Sharp [email: [e-mail]]]

You can file a new bug to cover the inconsistency. (It would be kind of annoying to fix, and I don't think it's a blocking issue.)