Closed
Bug 558651
Opened 15 years ago
Closed 14 years ago
"Check Spelling" should only be showed in context menu when input element is in 'text' or 'search' state
Categories
(Firefox :: Menus, defect)
Firefox
Menus
Tracking
()
VERIFIED
FIXED
Firefox 6
People
(Reporter: mounir, Assigned: mounir)
References
Details
(Keywords: html5)
Attachments
(1 file, 1 obsolete file)
977 bytes,
patch
|
sdwilsh
:
review+
|
Details | Diff | Splinter Review |
At the moment, the "Check Spelling" entry in context menu is showed for every input type with a text field. We should not show "Check Spelling" when the input type is 'url', 'email' or 'tel'.
In other words, we should show "Check Spelling" only for input element in 'text' and 'search' types.
Assignee | ||
Comment 1•15 years ago
|
||
This bug should be used to test context menus with new input types.
Assignee | ||
Comment 2•14 years ago
|
||
I have to check that the tests are fine on the try server given that they are disabled on Linux :(
Assignee | ||
Updated•14 years ago
|
Version: unspecified → Trunk
Assignee | ||
Updated•14 years ago
|
Attachment #490755 -
Flags: review? → review?(gavin.sharp)
Assignee | ||
Comment 3•14 years ago
|
||
The patch seems to work but I can't get the tests working (the attached ones have a few typos): the context menu have "Check Spelling" and even sometimes other menus related to dictionnaries. The only reason I see is the way the test work, InlineSpellCheckerUI.canSpell must always returns 'true' (uninit() isn't called?).
Gavin, would you be fine to have the patch pushed without the tests and add the tests later? I feel guilty to spend more time on that simple feature but I think it might be nice to have it.
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs review]
Updated•14 years ago
|
Assignee | ||
Comment 4•14 years ago
|
||
I've finally be able to find what was making the tests failing. It's because of a bug when there is an editable div, see bug 622283.
Because of that, I will have to create another file for the tests. I'm going to attach the one-liner patch here and open a new bug for a input context menu test file.
Assignee | ||
Comment 5•14 years ago
|
||
This patch is changing one line and updating a comment.
Attachment #490755 -
Attachment is obsolete: true
Attachment #500530 -
Flags: review?(gavin.sharp)
Attachment #490755 -
Flags: review?(gavin.sharp)
Assignee | ||
Comment 6•14 years ago
|
||
I have open bug 622284 for the tests.
Assignee | ||
Comment 7•14 years ago
|
||
Comment on attachment 500530 [details] [diff] [review]
Patch v1.1
Asking a review to dao in case of his review queue is smaller.
Attachment #500530 -
Flags: review?(dao)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs review] → [needs review][passed try]
Comment 8•14 years ago
|
||
Comment on attachment 500530 [details] [diff] [review]
Patch v1.1
r=sdwilsh
Feel free to punt the review of the tests my way too (I see it has a patch).
Attachment #500530 -
Flags: review?(gavin.sharp)
Attachment #500530 -
Flags: review?(dao)
Attachment #500530 -
Flags: review+
Assignee | ||
Updated•14 years ago
|
Whiteboard: [needs review][passed try] → [fixed in cedar]
Target Milestone: --- → Firefox 6
Comment 9•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Comment 10•14 years ago
|
||
Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:6.0a1) Gecko/20110505 Firefox/6.0a1
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•