1418629 - Spellcheck marks lost for words containing apostrophes

Status: RESOLVED FIXED

(Core :: Spelling checker, defect, P3)

Description

[ben.s.kuhn]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171112125346

Steps to reproduce:

1. Visit a page with a spellchecked text field, e.g. https://bugzilla.mozilla.org/enter_bug.cgi#h=dupes%7CFirefox
2. Focus the text field and type "doesn'"


Actual results:

The string "doesn" (without the final apostrophe) is underlined in red as if misspelled.


Expected results:

The string should not be underlined until I type a space or defocus the text field, as happens in Chrome or in Firefox with other incomplete words (e.g. "doesn" without the apostrophe).

Comment 1

[Ekanan Ketunuti]

Last good revision: Otto Länd 6d3323e4e071
First bad revision: Otto Länd 307fd37d519d
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6d3323e4e071&tochange=307fd37d519d

regressed by bug 1362858.

Comment 2

[Evelyn Hung]

Yes, it seems to be a side effect of my patch. Thanks for reporting this bug. I will look into it.

Comment 4

[Jorg K (CEST = GMT+2)]

Evelyn, any update?

I noticed the following: Type this into a https://www-archive.mozilla.org/editor/midasdemo/ and notice some funny effects:
calll's chichen<enter>

As soon as you type the "c" of chicken, "calll's" loses it's underline. That happens here or BMO, too. On Midas, the underline returns when you hit enter, on BMO it doesn't.

Losing spellcheck underlines is a great inconvenience to people who want to correct spelling mistakes later.

Comment 5

[Jorg K (CEST = GMT+2)]

Confirmed regression in TB, working on 2017-06-13, not working on 2017-06-14.

Comment 6

[Jorg K (CEST = GMT+2)]

Fixed for TB 60 ESR via backout:
https://hg.mozilla.org/releases/mozilla-esr60/rev/78c54e4a83e8807cdeaa96e22648d381eb51c8da on THUNDERBIRD_60_VERBRANCH
Backed out part of changeset eaf99ba3813a (bug 1362858, part 1, for causing bug 1418629) to build Thunderbird 60.2. a=jorgk

Comment 7

[Makoto Kato [:m_kato]]

(In reply to Jorg K (GMT+2) from comment #4)
> Evelyn, any update?

She is no longer paid stuff and inactive.

Comment 8

[Jorg K (CEST = GMT+2)]

Any plans to get this fixed by someone else then? The FF editor plus spell checker is used for entering text, for example on webmail sites, and it doesn't look so good that spellcheck marks just disappear :-( - As you saw, I backed this out so it wouldn't hit TB users who will be updated automatically to TB 60.2 soon. So far there wasn't an automated update from TB 52.

You can see by the backout that the problem can't be huge, some small logic error somewhere.

Comment 9

[Makoto Kato [:m_kato]]

(In reply to Jorg K (GMT+2) from comment #8)
> Any plans to get this fixed by someone else then? The FF editor plus spell
> checker is used for entering text, for example on webmail sites, and it
> doesn't look so good that spellcheck marks just disappear :-( - As you saw,
> I backed this out so it wouldn't hit TB users who will be updated
> automatically to TB 60.2 soon. So far there wasn't an automated update from
> TB 52.

OK, I will look this.

Comment 10

[Jorg K (CEST = GMT+2)]

Makoto-san, any news on this? We're already at mozilla66, so not much time to get this fixed in mozilla68 ESR.

Comment 11

[Makoto Kato [:m_kato]]

WordSplitState::ClassifyCharacter shouldn't return CHAR_CLASS_SEPARATOR.  I need write a test.

Comment 12

[Jorg K (CEST = GMT+2)]

Any update? We're coming to the end of mozilla67 and I'd really like to see this fixed for the next ESR.

Comment 13

[Jorg K (CEST = GMT+2)]

Jim, how can we get this fixed? I'm contacting you as the triage owner. The 68 cycle is coming to and end in about a month, so if this doesn't get fixed, it may be broken through the 68 ESR cycle like it was broken during the entire ESR 60 cycle.

This is very visible for users using FF for webmail for example. Try it here: Type "Thiss's" and see the effect. When you type the apostrophe, the Thiss is underlined, when you type the "s" the underlining goes. Quite annoying really.

Comment 14

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9436456b42016cfd5a0c709139f1f987e8cf3a3

Comment 15

[Jorg K (CEST = GMT+2)]

Thanks! :-)

Comment 16

[Jorg K (CEST = GMT+2)]

Any chance to get this finished before your PTO?

Comment 17

[Makoto Kato [:m_kato]]

Attachment: Bug 1418629 - Single quotation mark shouldn't always separator.

This seems to be regression by bug 1362858.

Actually, single quotation mark is always separator for spellchecker after
landing bug 1462858. When user tries to input "doesn't", "'" becomes separator
for spellchecker. Then "doesn" will be misspell word.

So we shouldn't mark single quotation mark as separator if user is inputting
word.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5822c9d23ff7
Single quotation mark shouldn't always separator. r=Ehsan

Comment 19

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/5822c9d23ff7

Comment 20

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/b62ef04e0947bf0d621cc87c2314ac3a4a660359 on THUNDERBIRD_68_VERBRANCH
Backed out changeset 5822c9d23ff7 (bug 1418629 for causing bug 1565919) to build Thunderbird 68.0. a=jorgk