Bug 62276 (suse47276)

<!-- SBZ_reproduce  -->
Run gnome-terminal, hit F1, go to the next page.  Note missing images.

Created attachment 25039 [details]
gnome-xml-support-for-kdoctools-part2.diff

Fixes the bug.	Named to match the patch already in kdelibs.  Also does some
language clean up.

Sorry, I missed the patch somehow. There are two things I don't understand. 
First is the strange slash loop - what is it trying to do? 
 
And then you remove the fallback - this for sure breaks KDE. 
-    langs.append( "en" ); 
-    langs.remove( "C" ); 

The strange slash loop is ignoring initial slashes.  It looks for the first non
slash character and then finds the first slash after that.  Specifically, it
walks along looking for the next slash character and only stops when the next
slash character is more than one byte forward.

I didn't just remove it, I reimplemented it below.

+        if ( *it == "en_US" || *it == "C")
             *it = "en";

This way, the "C" to "en" conversion happens in place in the string.  And you're
less likely to get dupliated "en".  I guess you might want the "en" to always be
there.  In that case, I would suggest checking if that *it = "en"; ever matched
and if not, then append "en".  Actually, doing the same for the GNOME section
might not be such a bad idea.  In fact, this processing could happen before the
GNOME section, and then the GNOME section would just be checking for "en" and
treating it as "C".

I think I understand what your loop does now. A comment might be in place 
though :) 
 
Right, the .append("en") is pretty unrelated to the .remove("C") - it's not a 
search+replace. If your lang list is "de_DE", "de", you still want to see 
something if there is no translated version - which is very likely for 
screenshots in KDE. 

Yeah, that totally does make sense.  I do think it's still useful to include "C"
in the search & replace below and just only add en if it wasn't there already. 
This way ordering would be preserved.

Would you be willing to make that change and then move this above the GNOME part
(so that the "en" gets added at the end before the GNOME processing (which of
course gets treated as "C", but...)) or should I do this and submit a new patch?

Poke? Ping?

WIP, I spend some minutes with it today and there still some issues.  
And I do not understand some attempts from the patch, but I had no real time 
yet to really work on it. 

Created attachment 25355 [details]
alternative patch

what about this patch ? it has little less risk of an endless loop and adds
fallback search pathes for different languages.

Would this one be okay ?

There's a couple things I don't like about this patch.

The one involving GNOME is that you're still losing ordering information for GNOME.  No matter how early "en" appears in the list, English GNOME docs won't be searched until the end.

Also, I think you broke KDE docs a bit.  You've removed the code to change en_US to en, which was getting used for KDE docs.  It seems like this will break KDE's stuff.  I think you want to add something like your language length > 2 thing in GNOME to the KDE section.

The change you made to the search for / seems okay.  The other code had 0 chance of an infinite loop because the index increased by 1 on every iteration of the loop.  The biggest difference with your loop is that it allocates a new string for every initial slash, doesn't it?  Also, I can't tell whether changing fname is safe without seeing all the code (I'm remote right now), but I'm guessing it's fine.

I'll review this patch in more depth once I am not remote.

the en_US -> en change does happen via the .left(2) line. 
 
I do not see, why there should be a double allocation. fname is anyway only a 
local variable, so it is no mem leak at least. (QString does anyway share the 
memory, if the strings are still equal). 

Created attachment 25376 [details]
kdoctools-picture-in-submenu.diff take two

After Stephan explained the problem to me ....

What about this patch, it should solve all issues ?

Created attachment 25421 [details]
third try

after discussion on irc.

I didn't tested this patch at all yet, does it look okay for you anyway ?

This looks pretty good, but the GNOME stuff should come before the lang list is modified (remove C, add en).  Is there some reason this can't happen?

the reason is that it doesn't change anything from the current behaviour in 
SLES9 in that way, so it is way more risk free to submit it in this way. 
Actually we would not even need the addedC flag and the last if append C line 
because of this. but let's stay that to be sure. 
 
Is it okay for you, when I do submit this now for SLES9 ? 

Except that right now in SLES9, the gnome reading stuff does come before the
lang list is modified.  If anything, by moving it below, you're changing the
sles9 behavior, aren't you?

Chris: can you please suggest a patch to got into 9.3 so we can get this bug 
fixed after NLD9 is done? 

This is a terrible bug in our docs.  We should get this into NLD9 SP2.

Since no one ever agreed on an acceptable patch, It hasn't been done to my
knowledge for STABLE, and this is a SLES package not an NLD package, I'm moving
this to future. SLES only takes blocker fixes after RC1.

Can any KDE maintainer comment on the patches? Does the fix look ok?

there is no final patch here and I'm not sure KDE didn't change too much in 
between 

I rechecked, its still broken for gnome-terminal and eog case for Code 10, see the "Getting Started" pages in both.  

ok, which patch in particular are we discussing for inclusion?

Chris is no longer @ximian

For the proposed inclusion patch, I'd say the KDE team needs to review the comments and assess again if any of the patches are even useable for code 10 and then just weight the merits since adrian and chris tried several approaches.

Oops, no longer needinfo

*** Bug 71672 has been marked as a duplicate of this bug. ***

Also in NLD9

*** Bug 265998 has been marked as a duplicate of this bug. ***

We won't fix that anymore for NLD and for code10 we don't use khelpcenter anymore for gnome docs, so the patch shouldn't be necessary at all.