Closed
Bug 637311
Opened 13 years ago
Closed 8 years ago
Checkmark not displayed for current encoding in View > Character Encoding when menu has been opened the first time
Categories
(Firefox :: Menus, defect, P4)
Tracking
()
RESOLVED
FIXED
Firefox 52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: fvsch, Assigned: stefanh)
Details
(Keywords: reproducible, Whiteboard: [4rc][tpi:+])
Attachments
(2 files)
257.91 KB,
image/png
|
Details | |
2.51 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
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•13 years ago
|
||
Comment 2•13 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
Comment 3•13 years ago
|
||
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
Comment 4•13 years ago
|
||
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
Comment 5•10 years ago
|
||
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.)
Comment 6•10 years ago
|
||
This is still present on OS X with a Nightly build from 20130110.
Comment 7•10 years ago
|
||
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
Comment 8•10 years ago
|
||
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.
Updated•10 years ago
|
Keywords: reproducible
Comment 9•10 years ago
|
||
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•8 years ago
|
||
I'm going to look at this since I have a theory about the problem.
Assignee: nobody → stefanh
Updated•8 years ago
|
Priority: -- → P4
Whiteboard: [4rc] → [4rc][tpi:+]
Assignee | ||
Comment 11•8 years 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•8 years ago
|
||
Assignee | ||
Comment 13•8 years ago
|
||
ni? myself in order to remember requesting review.
Flags: needinfo?(stefanh)
Assignee | ||
Updated•8 years ago
|
Status: NEW → ASSIGNED
Flags: needinfo?(stefanh)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(stefanh)
Assignee | ||
Comment 14•8 years 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•8 years 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•8 years 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•8 years ago
|
Component: Widget: Cocoa → Menus
Product: Core → Firefox
Comment 17•8 years 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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a468e41c48de
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
Comment 19•7 years ago
|
||
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.
Description
•