Last Comment Bug 558651 - "Check Spelling" should only be showed in context menu when input element is in 'text' or 'search' state
: "Check Spelling" should only be showed in context menu when input element is ...
: html5
Product: Firefox
Classification: Client Software
Component: Menus (show other bugs)
: Trunk
: All All
-- normal (vote)
: Firefox 6
Assigned To: Mounir Lamouri (:mounir)
: Jared Wein [:jaws] (please needinfo? me)
Depends on: 344615 456229 555559 557620
  Show dependency treegraph
Reported: 2010-04-11 05:13 PDT by Mounir Lamouri (:mounir)
Modified: 2011-05-06 05:35 PDT (History)
9 users (show)
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (4.30 KB, patch)
2010-11-15 16:46 PST, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v1.1 (977 bytes, patch)
2010-12-31 10:49 PST, Mounir Lamouri (:mounir)
sdwilsh: review+
Details | Diff | Splinter Review

Description User image Mounir Lamouri (:mounir) 2010-04-11 05:13:16 PDT
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.
Comment 1 User image Mounir Lamouri (:mounir) 2010-04-14 05:53:24 PDT
This bug should be used to test context menus with new input types.
Comment 2 User image Mounir Lamouri (:mounir) 2010-11-15 16:46:49 PST
Created attachment 490755 [details] [diff] [review]
Patch v1

I have to check that the tests are fine on the try server given that they are disabled on Linux :(
Comment 3 User image Mounir Lamouri (:mounir) 2010-11-16 04:31:38 PST
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.
Comment 4 User image Mounir Lamouri (:mounir) 2010-12-31 10:45:49 PST
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.
Comment 5 User image Mounir Lamouri (:mounir) 2010-12-31 10:49:45 PST
Created attachment 500530 [details] [diff] [review]
Patch v1.1

This patch is changing one line and updating a comment.
Comment 6 User image Mounir Lamouri (:mounir) 2010-12-31 10:59:41 PST
I have open bug 622284 for the tests.
Comment 7 User image Mounir Lamouri (:mounir) 2011-01-26 10:03:35 PST
Comment on attachment 500530 [details] [diff] [review]
Patch v1.1

Asking a review to dao in case of his review queue is smaller.
Comment 8 User image Shawn Wilsher :sdwilsh 2011-05-02 13:08:05 PDT
Comment on attachment 500530 [details] [diff] [review]
Patch v1.1


Feel free to punt the review of the tests my way too (I see it has a patch).
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2011-05-04 13:55:24 PDT
Comment 10 User image Simona B [:simonab ] 2011-05-06 05:35:18 PDT
Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:6.0a1) Gecko/20110505 Firefox/6.0a1

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