The default bug view has changed. See this FAQ.

Checkmark not displayed for current encoding in View > Character Encoding when menu has been opened the first time

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Menus
P4
minor
RESOLVED FIXED
6 years ago
a month ago

People

(Reporter: Florens Verschelde, Assigned: stefanh)

Tracking

({reproducible})

Trunk
Firefox 52
All
Mac OS X
reproducible
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [4rc][tpi:+])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12) Gecko/20100101 Firefox/4.0b12
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12) Gecko/20100101 Firefox/4.0b12

When using the View > Character Encoding menu for the first time in a Firefox session, or for the first time after opening a new window, Firefox does not display a checkmark before the currently used encoding.
The View > Character Encoding > Auto-Detect menu has the same issue.

I'll attach a screenshot.

Problem seen on OS X with FF4b12.
Seen by a friend on Archlinux i686 with FF4b11.

Reproducible: Always

Steps to Reproduce:
1. Start Firefox and open any page.
2. Go to the View > Character Encoding menu
Actual Results:  
The current encoding (whether declared or auto-detected) should be shown with a check mark.

Expected Results:  
No encoding is highlighted.
(Reporter)

Comment 1

6 years ago
Created attachment 515608 [details]
Combined screenshots

Comment 2

6 years ago
Able to reproduce using  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b13pre) Gecko/20110228 Firefox/4.0b13pre.
Status: UNCONFIRMED → NEW
Ever confirmed: true
That's a regression. Would be great to have a regression date available. Teodosia, would you have time for such a check?
Severity: minor → normal
Keywords: regression, regressionwindow-wanted
Hardware: x86 → All
Whiteboard: [4rc]
Version: unspecified → Trunk
As a quick check shows it only happens the first time the menu has been used. Each successive attempt will succeed.
Severity: normal → minor
Summary: Checkmark not displayed for current encoding in View > Character Encoding → Checkmark not displayed for current encoding in View > Character Encoding when menu has been opened the first time
Works for me on trunk on Linux. Is this bug still present on Mac? (Note that as of Firefox 28, some unusual encodings are no longer in the menu, so you need to test with the page that uses one of the encodings that are still in the menu.)
This is still present on OS X with a Nightly build from 20130110.
OK. This is then probably a bug in how XUL menus are reflected as Mac native menus rather than a bug in the Character Encoding menu implementation.
Component: Menus → Widget: Cocoa
Product: Firefox → Core
I can reproduce this if FF 26 on OS X 10.8.5.  How bizarre!

But as this is just a minor annoyance, it may take a while for somebody to get to it.
Keywords: reproducible
This is so old it's no longer really a "regression".  That doesn't mean we shouldn't fix it, though.
Keywords: regression, regressionwindow-wanted
(Assignee)

Comment 10

6 months ago
I'm going to look at this since I have a theory about the problem.
Assignee: nobody → stefanh

Updated

5 months ago
Priority: -- → P4
Whiteboard: [4rc] → [4rc][tpi:+]
(Assignee)

Comment 11

5 months ago
So the problem here is that we set the checkmark after the menu have been shown: https://dxr.mozilla.org/comm-central/rev/5639a9f476d08f300c079117e61697f5026b6367/mozilla/browser/base/content/browser-charsetmenu.inc#9-10.

That doesn't work on Mac since you can't update a Mac menu when it's visible. The obvious solution is to call UpdateCurrentCharset(this) when the popupshowing event fires (after CharsetMenu.build(event.target)).

Gijs isn't back until the 26:th, but I'll attach a patch that he can look at when he's back.
(Assignee)

Comment 12

5 months ago
Created attachment 8803650 [details] [diff] [review]
Update charset menu before it's visible
(Assignee)

Comment 13

5 months ago
ni? myself in order to remember requesting review.
Flags: needinfo?(stefanh)
(Assignee)

Updated

5 months ago
Status: NEW → ASSIGNED
Flags: needinfo?(stefanh)
(Assignee)

Updated

5 months ago
Flags: needinfo?(stefanh)
(Assignee)

Comment 14

5 months ago
Comment on attachment 8803650 [details] [diff] [review]
Update charset menu before it's visible

Gijs, mind take a look at this? Info is in comment #11. One alternative is to only change this for mac (ifdef).
(Assignee)

Comment 15

5 months ago
Comment on attachment 8803650 [details] [diff] [review]
Update charset menu before it's visible

Gijs, mind take a look at this? Info is in comment #11. One alternative is
to only change this for mac (ifdef).

(this time with a review request)
Flags: needinfo?(stefanh)
Attachment #8803650 - Flags: review?(gijskruitbosch+bugs)

Comment 16

5 months ago
Comment on attachment 8803650 [details] [diff] [review]
Update charset menu before it's visible

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

Apparently this has been iffy since at least Firefox 4 (bug 598006). Huh.
Attachment #8803650 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Updated

5 months ago
Component: Widget: Cocoa → Menus
Product: Core → Firefox

Comment 17

5 months ago
Pushed by stefanh@inbox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a468e41c48de
Update charset menu before it's visible so the checkmark for current encoding is displayed on Mac the first time the menu is opened. r=Gijs.

Comment 18

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a468e41c48de
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Verified fixed on Fx 52.0b7 Ubuntu 16.04
You need to log in before you can comment on or make changes to this bug.