Issue 98275 - Opening htm doc with notes brings up spellcheck dialog box
Summary: Opening htm doc with notes brings up spellcheck dialog box
Alias: None
Product: Writer
Classification: Application
Component: ui (show other issues)
Version: DEV300m38
Hardware: All All
: P3 Trivial (vote)
Target Milestone: ---
Assignee: eric.savary
QA Contact: issues@sw
Depends on:
Reported: 2009-01-20 11:08 UTC by wolframgarten
Modified: 2013-08-07 14:43 UTC (History)
5 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---

bugdoc (502 bytes, text/html)
2009-01-20 11:13 UTC, wolframgarten
no flags Details
patch (356 bytes, text/plain)
2009-01-23 12:40 UTC, max.odendahl
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description wolframgarten 2009-01-20 11:08:46 UTC
When opening a html file with notes a dialog box comes up: Continue checking at
beginning of document? This does not happen with m38 master.
Comment 1 wolframgarten 2009-01-20 11:09:35 UTC
Adding mba and sba to cc.
Comment 2 wolframgarten 2009-01-20 11:13:25 UTC
Created attachment 59515 [details]
Comment 3 wolframgarten 2009-01-20 11:14:11 UTC
Version is m39 but this is not selectable yet in dropdown box.
Comment 4 max.odendahl 2009-01-20 14:33:32 UTC
mod->tl: seems like I need your help for this one, as sth. appearantly changed
in the framework.

This error comes up when calling View()->StartSpeller() inside
SwMarginWin::InitControls() in sw/source/ui/docvw/postit.cxx. View() is the
EditView of the notes, I am using this code since ages to start the spellcheck
inside notes.

It works fine for regular documents, so sth. seems to go different for html docs
Comment 5 thomas.lange 2009-01-21 12:42:15 UTC
tl->mod: since I'm not familar with notes: where is the code that starts the
automatic spell checking for notes?
Just curious since I think the problem to solve is not to explicitly suppress
that dialog, but probably to check why spell checking is started so early.
Comment 6 max.odendahl 2009-01-21 13:16:02 UTC
as I wrote in the comment before, it is View()->StartSpeller() inside
SwMarginWin::InitControls() in sw/source/ui/docvw/postit.cxx. View() is the
EditView of the notes
Comment 7 thomas.lange 2009-01-21 14:56:05 UTC
tl->mod: OOps sorry about the location I missed that when looking into this issue.
But in that case: why is it started there? Is there a specific reason?
Usually I would have expected it to be started in the very same location where
it is done for the automatic checking in the main diocument and where grammar
checking is also started now.
That is in doclay.cxx within the idle loop:
IMPL_LINK( SwDoc, DoIdleJobs, Timer *, pTimer )

somewhere around the lines:
        if (GetRootFrm()->IsNeedGrammarCheck())
                StartGrammarChecking( *this, *GetRootFrm() );

OR in layact.cxx in the function
SwLayIdle::SwLayIdle( SwRootFrm *pRt, SwViewImp *pI ) :

somewhere around
    if ( !DoIdleJob( SMART_TAGS, TRUE ) &&
         !DoIdleJob( ONLINE_SPELLING, TRUE ) &&

Doing it in SwMarginWin::InitControls before(!) the window gets displayed is
most probably a bad idea. The spelling should only be started when the document
is already completely loaded.
Starting it in one of the two above locations should be a better place and might
already fix the problem.

Comment 8 max.odendahl 2009-01-21 15:35:31 UTC
mod->tl: I tried putting the StartSpeller call for the notes into 

IMPL_LINK( SwDoc, DoIdleJobs, Timer *, pTimer )

but then I get the error message all the time, so I guess this is not the issue.

- as this worked before, I really wonder what has change in the spelling
framework and why it would work for odt-notes, but not html notes.

- SwSpellDialogChildWindow::GetNextWrongSentence is the initiator of this
dialog, so maybe that was changed and needs to be fixed again, e.g. by including
or excluding notes (SHELL_MODE_POSTIT)?
Comment 9 thomas.lange 2009-01-21 15:53:00 UTC
tl->mod: Ok. In that I case I will debug tomorrow to see who is calling that
dialog and why.

Still the issue remains, that the spelling should not be started before the view
is displayed (or better) the document is already loaded.
Comment 10 thomas.lange 2009-01-22 11:36:08 UTC
tl->mod: The difference between OOO300_m15 (working) and DEV300_m39 lies with in
your code.

the stack for m15 is:

 	svxmi.dll!ImpEditEngine::Spell(EditView * pEditView=0x09e50cb0, unsigned char
bMultipleDoc=0x00)  Line 1492	C++
 	svxmi.dll!EditView::StartSpeller(unsigned char bMultipleDoc=0x00)  Line 944	C++
 	svxmi.dll!OutlinerView::StartSpeller(unsigned char bMultiDoc=0x00)  Line 1379	C++
 	swmi.dll!SwPostIt::InitControls()  Line 787	C++
 	swmi.dll!SwPostIt::SwPostIt(Window * pParent=0x05b18908, __int64
nBits=0x0000000000000144, SwFmtFld * aField=0x09dbb798, SwPostItMgr *
aMgr=0x07994330, __int64 aBits=0x0000000000000000)  Line 512	C++
>	swmi.dll!SwPostItMgr::LayoutPostIts()  Line 589 + 0x5d bytes	C++
 	swmi.dll!SwView::OuterResizePixel(const Point & rOfst={...}, const Size &
rSize={...})  Line 1264	C++
 	sfxmi.dll!SfxViewFrame::DoAdjustPosSizePixel(SfxViewShell * pSh=0x07afcb48,
const Point & rPos={...}, const Size & rSize={...})  Line 1937	C++
 	sfxmi.dll!SfxViewFrame::Resize(unsigned char bForce=0x00)  Line 3040	C++
 	sfxmi.dll!SfxTopViewWin_Impl::Resize()  Line 382 + 0xd bytes	C++

the stack for m39 is:

 	svxmi.dll!SvxSpellWrapper::SpellNext()  Line 436	C++
 	svxmi.dll!SvxSpellWrapper::FindSpellError()  Line 634 + 0xb bytes	C++
 	svxmi.dll!SvxSpellWrapper::SpellDocument()  Line 407 + 0x8 bytes	C++
 	svxmi.dll!ImpEditEngine::Spell()  + 0x170 bytes	C++
 	svxmi.dll!EditView::StartSpeller()  + 0x9a bytes	C++
 	svxmi.dll!OutlinerView::StartSpeller()  + 0x27 bytes	C++
 	swmi.dll!SwMarginWin::InitControls()  Line 814	C++
>	swmi.dll!SwPostItMgr::LayoutPostIts()  Line 636	C++
 	swmi.dll!SwView::OuterResizePixel(const Point & rOfst={...}, const Size &
rSize={...})  Line 1264	C++
 	sfxmi.dll!SfxViewFrame::DoAdjustPosSizePixel()  + 0x59 bytes	C++
 	sfxmi.dll!SfxViewFrame::Resize()  + 0xb9 bytes	C++
 	sfxmi.dll!SfxTopViewWin_Impl::Resize()  + 0x3c bytes	C++

And the code difference is in PostItMgr.cxx where the lines 

						if (pItem->bShow)
							long Y = mpEditWin->LogicToPixel( Point(0,pItem->mPos.Bottom())).Y();
							long aPostItHeight = 0;
							if (!pPostIt)
								pPostIt = new
								pItem->pPostIt = pPostIt;

got changed to:

						if (pItem->bShow)
							long Y = mpEditWin->LogicToPixel( Point(0,pItem->mPos.Bottom())).Y();
							long aPostItHeight = 0;
							if (!pPostIt)
								pPostIt =
								pItem->pPostIt = pPostIt;
								if (mpAnswer)
									if (pPostIt->CalcFollow()) //do we really have another note in front of
this one
									delete mpAnswer;
									mpAnswer = 0;

That is it is probably your new added call to pPostIt->InitControls() where you
start the speller that is making the trouble.

I asked AMA about his opinion on this. He also said that on of the previously
listed code lines related to the idle-loop should be the correct places to start
the spelling for notes as well.

But if you still want to do it in the current manner you may try if it helps to
*suppress starting the speller* when the document is not yet loaded. However in
that case it is unclear to if and who will start the speller later on...
If you want to check for the document still being loaded AMA suggested to try by
checking the 'mbInReading' member in SwDoc. If that is still true spelling will
be a bad idea. But it might even be necessary to wait until a layout is
available see 'pLayout' member in SwDoc.

I hope these informations will be of use to you.

Comment 11 thomas.lange 2009-01-22 11:36:53 UTC
tl->mod: back to you to decide in which manner you want to proceed.
Comment 12 thomas.lange 2009-01-22 11:38:59 UTC
Comment 13 max.odendahl 2009-01-22 12:56:38 UTC
mod->tl: as you can see in the two stacks, InitControls was also called before,
but inside of the constructor, now it is called right after the constructor by
the explicit call, so nothing really changed.

 - so StartSpeller was always used and appearantly works in 3.0.1, but not
anymore in m39.

- still wondering why it would work for regular notes and not for html notes

btw, I tried as mentioned to start the spelling inside the other places, but
then I get the same error message, so sth. else seems to be broken and not the
call to StartSpeller.
Comment 14 thomas.lange 2009-01-23 09:31:22 UTC
Debugging further into this the difference is quite the other way around as
expected before. In m39 the text for the notes seems to be already loaded at
that time (the first call to the speller) where it is not done in m15.

The lines where the difference occurs is in svx/source/editeng/impedit4.cxx in
the function  ImpEditEngine::Spell( EditView* pEditView, sal_Bool bMultipleDoc )
The respective lines are:

	sal_Bool bIsStart = sal_False;
	if ( bMultipleDoc )
		bIsStart = sal_True;	// Immer von Vorne bzw. von hinten...
    else if ( ( CreateEPaM( aEditDoc.GetStartPaM() ) == pSpellInfo->aSpellStart ) )
		bIsStart = sal_True;

In m15 pSpellInfo->aSpellStart is 0/0 and thus in the next line bIsStart will
become true. Both of these values are needed for the function

  EditSpellWrapper::SpellStart in edtspell.cxx to set the SpellInfo to values
that later on (in a different function) results in the decision that nothing
needs to be done right now. The values will be set in these lines:

        if ( !IsStartDone() )
			pSpellInfo->bSpellToEnd = sal_True;
			pSpellInfo->aSpellTo = pImpEE->CreateEPaM(
					pImpEE->GetEditDoc().GetEndPaM() );
			pSpellInfo->bSpellToEnd = sal_False;
			pSpellInfo->aSpellTo = pSpellInfo->aSpellStart;
					pImpEE->GetEditDoc().GetEndPaM() );

To prove my point:
If in m39 you go back to code lines in impedit4.cxx and set a breakpoint right
after those 5 lines got executed and then set the values to
  pSpellInfo->aSpellStart = 0/0
  bIsStart = sal_True
then even in m39 the problem will not occur.

I have not further debugged where in m39 pSpellInfo->aSpellStart gets its value
of 0/9 since I'm quite confident that it is the case because the notes text are
already loaded.

Thus it seems the reason is that in m15
            which calls mpOutlinerView->StartSpeller()
Thus SetPostItText was called AFTER StartSpeller() while in m39 it is the other
way around:
Now SetPostItText() is called before StartSpeller().
Comment 15 thomas.lange 2009-01-23 09:34:56 UTC
Of course the changed order of the calls might not be the problem itself maybe
by changing to the current order you just discovered a bug that was
hidden/unknown before...
Comment 16 thomas.lange 2009-01-23 09:43:05 UTC
tl->mod: Just another idea you may give a try.
Since the problem is the dialog that asks for confirmation of the wrap-around
and probably not the spelling in itself you may want to check what happens if
you set the EditEngine's cursor back to 0/0 BEFORE you start the speller. In
that case even if the whole text is to be spelled no wrap-around should be
necessary. Thus it may fix your problem.
Comment 17 max.odendahl 2009-01-23 12:09:30 UTC
mod->tl: thanks a lot, inserting the text after StartSpeller() and calling
CompleteOnlineSpelling() does not give me the warning as well as wrong text is
underlined after opening the document

Any idea why it works fine for normal, e.g. odf notes? There the text was
inserted first as well and then speller was started
Comment 18 thomas.lange 2009-01-23 12:30:35 UTC
tl->mod: no idea. Maybe it has to do with the details of the specific filter and
the order of things done there. 
Comment 19 max.odendahl 2009-01-23 12:39:18 UTC
as discussed
Comment 20 max.odendahl 2009-01-23 12:40:18 UTC
Created attachment 59624 [details]
Comment 21 thomas.lange 2009-01-23 12:50:54 UTC
tl->mod: Some comment about the odd and probably unexpected order of things
would have been nice...
Comment 22 max.odendahl 2009-01-23 12:56:09 UTC
mod->tl: not sure what you mean, a comment here or inside the code?

Any why odd or unexpected? 

If I am correct, the only way to start the spellchecking inside the Engine is
the StartSpeller call, it should not matter if I add text before or afterwards.
It works as well for normal notes, so this still looks like a bug to me, but
thanks to your help, we found a workaround
Comment 23 max.odendahl 2009-01-23 13:02:51 UTC
using ->StartSpeller(true) instead of ->StartSpeller() would have also worked
Comment 24 thomas.lange 2009-01-23 13:09:55 UTC
The odd thing (to people not knowing the context) is that the speller is called
before the text is set. Thus a reference to the issue would be fine.
Or maybe if StartSpell woul be named InitSpeller.
StartSpeller seems to imply that in course of that call spell checking will be
done. And if that were true it would look strange to add the text after the
spell checking was already started.

Thus either add a reference to the issue or rename the functions like
  StartSpeller -> InitSpeller
  CompleteOnlineSpelling -> DoOnlineSpelling
Choose whichever one you think is more appropriate.

The 2nd thing that looks a strange is that the object where spelling is started
is different from the one where it is ended (if that is the case).
Ideally, if those two function above do actually serve some start/stop logic for
a single specific task, one would expect them to be implemented in the very same
object. But since I do not know what they exactly do I cannot really comment on
this detail.
Comment 25 max.odendahl 2009-01-23 13:35:30 UTC
well, I can't rename this stuff. This is nothing I created, it is all old
functionality from EditView/EditEngine.

I guess this also explains your second observation

@mba: if you integrate the patch, a reference to this issue might make sense anyway
Comment 26 max.odendahl 2009-01-23 13:37:42 UTC
>>But since I do not know what they exactly do I cannot really comment on
>>this detail.

aren't you the spelling/grammar guy as well as the one responsible for
EditView/EditEngine? if not you, who would know?
Comment 27 thomas.lange 2009-01-23 13:52:04 UTC
tl->mod: If you ever 'inherited' a bunch of code that you did not wrote, that is
not comment, that sometimes acts strange (compared to how you would think it
should work), and where you only once-in-a-while need/can look into when the
house is already on fire in rooms you have barely or never seen before, then you
would understand me a bit more. ^_-
Comment 28 max.odendahl 2009-01-23 13:55:09 UTC
ok, points taken.....

In relation to that, I have several other issues for which exists issues in
relation to EditView/EditEngine. Am I right that when I would like to get them
solved, I would need to do this by myself?
Comment 29 thomas.lange 2009-01-23 14:06:32 UTC
tl->mod: not necessarily. As you already stated, I'm the one responsible for the
EditEngine/EditView. But which issues/problems I am to to take care of is
usually decided in agreement with my manager according to the severity and needs
and other high priority issues at hand.
Of course we are always willing to take the time to give advice!
And we can't really expect someone to work on the OOo code without doing that. :-)
Thus if you have problems, just ask I will always have a look into the things
and give advise or cross check with the original developer which still lurks around.
But if I got higher priority tasks assigned it may have to wait until later if I
cannot come up with some help within a short time frame.

Be assured that if someone is willing to work on the code we are also willing to
give as much a helping hand as is possible.
Comment 30 max.odendahl 2009-01-23 14:11:33 UTC
>>Be assured that if someone is willing to work on the code we are also willing to
>>give as much a helping hand as is possible.

yes, of course I know that and I am very thankful for it, I do not want to
create the wrong impression in this issue for sure. Just wondering as the
EditView stuff is pushed back further and further.

Thanks for your help, let's not further fill up the issue, I'll follow up on the
other EditEngine/EditView stuff in more appropriate places.
Comment 31 Mathias_Bauer 2009-01-29 14:46:03 UTC
fixed in cws notes8
Comment 32 Mathias_Bauer 2009-01-29 14:46:34 UTC
please verify
Comment 33 eric.savary 2009-01-29 15:02:07 UTC
Verified in CWS notes8
Comment 34 max.odendahl 2009-02-13 01:26:26 UTC