Closed Bug 337845 Opened 18 years ago Closed 18 years ago

Spell checking options should be at the bottom of the context menu

Categories

(Firefox :: General, defect)

2.0 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 2 beta2

People

(Reporter: zeniko, Assigned: zeniko)

References

Details

(Keywords: fixed1.8.1)

Attachments

(1 file)

Since (de)activating the inline spell checker isn't the most often used feature of the context menu and since most text controls (at least under Windows) start with "Undo, Cut, Copy, Paste", I suggest moving the spell checker entries down - below "Select All". Spelling suggestions probably can stay at the top though.

Steps to reproduce:
1. Type some text and select it
2. Right-click and click on the fourth menu item (muscle memory for copying)

Expected result:
The selected text is copied to the clipboard.

Actual result:
The typed text disappeared (since the fourth item is now Undo).
Flags: blocking-firefox2?
Not a blocker. Here's a mockup of how it should look .. 

   .-----------------------.
   | few                   |
   | foot                  |
   | fool                  |
   | food                  |
   | too                   |
   |-----------------------|
   | Ignore                |  <-- this will need another bug, I think :)
   | Add to dictionary     |
   |-----------------------|
   | Undo                  |
   |-----------------------|
   | Cut                   |
   | Copy                  |
   | Paste                 |
   | Delete                |
   |-----------------------|
   | Select All            |
   |-----------------------|
   |xSpell check this field|
   | Languages            >|
   '-----------------------'
Flags: blocking-firefox2? → blocking-firefox2-
I'm still a little on the fence on this, design-wise. Most apps with spellchecking only show the options when something is mis-spelled, but that's because spellchecking is either "on" or "off" pretty much globally. We have the "Spellcheck this field" option which seems like a less common user task than "Cut, Copy, Undo, Paste" yet should probably be co-located with the rest of the spelling UI.

Need to ruminate for a while. Brett, your thoughts would be appreciated here.
I think on/off and dictionary selection should be at the bottom. I used to think they should be all together (that's why it's this way now). But I think the context menu if you don't click on a misspelled word is not very good, and some people use it a lot.

I think "Ignore" is overkill, and the menu is already getting too big, but we can have that discussion later.

I wonder if we can get rid of the divider between the suggestions and "Add to dictionary"/"Ignore". The suggestions are bold, which might provide sufficient differentiation. Then the first section is "thing you can do with this misspelling", while the bottom section is "control spellcheck for this field."

I won't get around to it for a while, but it's easy to do at any point in the future. If any newcomer to help, it should be easy. The menu XUL and the code that turns on and off the spellchecking options need to be modified. Contact me for more info.
Whiteboard: [good first bug]
- Agreed, ignore is for another bug ;)

- I'd also played with the "Add to dictionary" in the same section as the spelling terms, would be fine with that, I think.
Attached patch fixSplinter Review
This patch removes the divider between the search suggestions and "Add to dictionary" and otherwise makes the menus look as suggested by Mike.

Brett: Is it intentional that "Add to dictionary" and "Spell check this field" aren't properly capitalized (opposed to e.g. "Select All")?
Attachment #229045 - Flags: review?(brettw)
Status: NEW → ASSIGNED
Whiteboard: [good first bug] → [has patch][needs review brettw]
Target Milestone: --- → Firefox 2 beta2
Comment on attachment 229045 [details] [diff] [review]
fix

Looks good. Sorry it took so long.
Attachment #229045 - Flags: review?(brettw) → review+
Whiteboard: [has patch][needs review brettw] → [has patch]
Assignee: nobody → zeniko
Status: ASSIGNED → NEW
Whiteboard: [has patch] → [has patch][checkin needed]
Whiteboard: [has patch][checkin needed] → [has patch][needs sr]
Attachment #229045 - Flags: superreview?(bugs)
Whiteboard: [has patch][needs sr] → [has patch][checkin needed]
mozilla/browser/base/content/browser-context.inc 	1.24
mozilla/browser/base/content/browser.js 	1.669
mozilla/toolkit/content/widgets/textbox.xml 	1.37
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][checkin needed] → [needs approval]
Attachment #229045 - Flags: approval1.8.1?
Comment on attachment 229045 [details] [diff] [review]
fix

approved by schrep for drivers
Attachment #229045 - Flags: approval1.8.1? → approval1.8.1+
Whiteboard: [needs approval] → [checkin needed]
Whiteboard: [checkin needed] → [checkin needed (1.8 branch)]
mozilla/toolkit/content/widgets/textbox.xml 	1.21.4.11
mozilla/browser/base/content/browser-context.inc 	1.14.2.9
mozilla/browser/base/content/browser.js 	1.479.2.177
Keywords: fixed1.8.1
Whiteboard: [checkin needed (1.8 branch)]
Version: unspecified → 2.0 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: