Last Comment Bug 575182 - Cannot change Character Encoding from Firefox Menu Button
: Cannot change Character Encoding from Firefox Menu Button
Status: RESOLVED FIXED
: intl, jp-critical
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: ---
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on: 596173 598006 616836
Blocks: FirefoxButton
  Show dependency treegraph
 
Reported: 2010-06-27 23:27 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2014-04-25 15:15 PDT (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta7+


Attachments
patch (7.87 KB, patch)
2010-09-01 11:34 PDT, :Margaret Leibovic
faaborg: ui‑review+
Details | Diff | Splinter Review
patch v2 (15.15 KB, patch)
2010-09-01 14:01 PDT, :Margaret Leibovic
gavin.sharp: review+
Details | Diff | Splinter Review
final patch (15.53 KB, patch)
2010-09-02 11:53 PDT, :Margaret Leibovic
no flags Details | Diff | Splinter Review
final patch (should apply now) (15.56 KB, patch)
2010-09-07 11:45 PDT, :Margaret Leibovic
no flags Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2010-06-27 23:27:31 PDT
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] (Mozilla Japan) 2010-06-27 23:35:17 PDT
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) 2010-07-12 02:31:04 PDT
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 [:bsmedberg] 2010-07-19 08:31:38 PDT
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 2010-07-29 07:15:31 PDT
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] 2010-07-29 14:00:19 PDT
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) 2010-07-29 15:58:15 PDT
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] 2010-07-30 06:56:19 PDT
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) 2010-07-30 14:59:17 PDT
>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] 2010-07-30 15:43:15 PDT
Not sure that Limi is the best assignee for this bug.
Comment 10 Alex Faaborg [:faaborg] (Firefox UX) 2010-07-30 16:24:44 PDT
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 2010-08-12 09:36:43 PDT
(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.

*** This bug has been marked as a duplicate of bug 583386 ***
Comment 12 Masayuki Nakano [:masayuki] (Mozilla Japan) 2010-08-21 07:57:13 PDT
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 2010-08-25 15:19:33 PDT
(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] (Mozilla Japan) 2010-08-25 20:08:57 PDT
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 2010-08-26 17:50:55 PDT
(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) 2010-08-26 18:57:48 PDT
>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] (Mozilla Japan) 2010-08-26 19:03:08 PDT
(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) 2010-08-26 19:31:37 PDT
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] 2010-08-26 19:57:43 PDT
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) 2010-08-27 12:12:48 PDT
>Who's doing that work?

Johnath: this bug could use an owner
Comment 21 Michael Lefevre 2010-09-01 02:43:00 PDT
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] 2010-09-01 07:59:46 PDT
Yes. I'm also just going to assign it so someone, eenie meenie miney Margaret!
Comment 23 :Margaret Leibovic 2010-09-01 08:58:59 PDT
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 2010-09-01 11:34:43 PDT
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?

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 2010-09-01 12:36:34 PDT
(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 2010-09-01 14:01:31 PDT
Created attachment 471275 [details] [diff] [review]
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) 2010-09-01 14:29:33 PDT
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: gavin@gavinsharp.com] 2010-09-02 11:03:56 PDT
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 2010-09-02 11:53:41 PDT
Created attachment 471576 [details] [diff] [review]
final patch

Fixed license block and tabs.
Comment 30 Dão Gottwald [:dao] 2010-09-04 01:56:11 PDT
patching file browser/base/content/browser.xul
Hunk #1 FAILED at 566
Comment 31 :Margaret Leibovic 2010-09-07 11:45:18 PDT
Created attachment 472706 [details] [diff] [review]
final patch (should apply now)

This should apply after the patch for bug 589139 lands.
Comment 32 :Margaret Leibovic 2010-09-07 14:27:33 PDT
http://hg.mozilla.org/mozilla-central/rev/7da00dcb512e
Comment 33 Vlado Valastiak [:wladow] @ Mozilla.sk 2010-09-08 10:06:23 PDT
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) 2010-09-08 14:47:24 PDT
alt = normal menu
alt-f = focused firefox menu (planned), followed by arrow key access to the sub-menu.
Comment 35 Vlado Valastiak [:wladow] @ Mozilla.sk 2010-09-09 09:20:37 PDT
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: gavin@gavinsharp.com] 2010-09-11 23:35:08 PDT
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.)

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