Issue 98275

Adding mba and sba to cc.

Created attachment 59515 [details]
bugdoc

Version is m39 but this is not selectable yet in dropdown box.

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

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.

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

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 ) &&
         !DoIdleJob( AUTOCOMPLETE_WORDS, TRUE ) ) // SMARTTAGS

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.

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)?

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.

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
SwPostIt(static_cast<Window*>(&mpView->GetEditWin()),WINDOW_CONTROL,pFmtFld,this,0);
								pPostIt->SetReadonly(mbReadOnly);
								SetColors(pPostIt,static_cast<SwPostItField*>(pFmtFld->GetFld()));
								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 =
(*i)->GetMarginWindow(static_cast<Window*>(&mpView->GetEditWin()),WINDOW_CONTROL,this,0);
								pPostIt->InitControls();
								pPostIt->SetReadonly(mbReadOnly);
								pItem->pPostIt = pPostIt;
								if (mpAnswer)
								{
									if (pPostIt->CalcFollow()) //do we really have another note in front of
this one
										static_cast<SwPostIt*>(pPostIt)->InitAnswer(mpAnswer);
									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.

tl->mod: back to you to decide in which manner you want to proceed.

.

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.

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() );
		}
		else
		{
			pSpellInfo->bSpellToEnd = sal_False;
			pSpellInfo->aSpellTo = pSpellInfo->aSpellStart;
			pEditView->GetImpEditView()->SetEditSelection(
					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
and 
  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
	InitControls();
            which calls mpOutlinerView->StartSpeller()
	SetPostItText();
Thus SetPostItText was called AFTER StartSpeller() while in m39 it is the other
way around:
	SetPostItText();
	SetLanguage(GetLanguage());
	View()->StartSpeller();
Now SetPostItText() is called before StartSpeller().

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...

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.

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

tl->mod: no idea. Maybe it has to do with the details of the specific filter and
the order of things done there. 

as discussed

Created attachment 59624 [details]
patch

tl->mod: Some comment about the odd and probably unexpected order of things
would have been nice...

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

using ->StartSpeller(true) instead of ->StartSpeller() would have also worked

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
and
  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.

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

>>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?

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. ^_-

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?

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.

>>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.

fixed in cws notes8

please verify

Verified in CWS notes8

closing