Created attachment 28992 [details]
bad rendering of slashdot.org

I have the same issue with Konqueror and the new slashdot style, overlapping text.

Version: 3.5.10 (Gentoo)
Compiler gcc-4.3.2
OS: Linux (x86) 2.6.26-gentoo-r3

Happens to me too with KDE 3.5.10 on Archlinux...

Created attachment 31860 [details]
possible testcase

not sure if this is the exact scenario, but likely.

it works in gecko, presto, not wk.
we need to somehow un-blockify renderers with originalDisplayInlineType when a style change makes it possible.

(good lord, my eyes are bleeding.. tracing through humongous hypes of bloated, poorly written jQuery-driven code is a living *hell* - if this is the future, we are so screwed)

better analysis : hosting block inside inlines creates continuations (which are an ad hoc parallel structure we should get rif of sooner rather than later).

The problem is that when such blocks are removed, the continuations are not folded back. We need to perform that surgery, as much as we can.

Longterm, we need to remove continuations, and replace them with a real container for block in inlines.

Created attachment 31915 [details]
patch for the continuation issue

an update on this...
the attachment will fix the so called 'tags' being on top of another on slashdot.

I'm now working on the small triangle that should appear to the left of the tags. 
I'm mid way through it but this is a tougher nut to crack, as it needs a rework of our line breaking model to be done correctly (right now, inline flows are only accounted for when they have some text content - this is wrong because they may have some padding/border/margin ; beside their right and left b/p/m width is always added to the line width, and this is bogus because one of the side might be on another line).

I saw WebCore has a quick hack just for the slashdot triangle, but it's not adressing the real issue - as shown on the test case I'll attach in a moment.

Created attachment 31917 [details]
testcase for line breaking and rendering of inline borders/padding/margin

SVN commit 939175 by ggarand:

merge continuations back into a single inline when the block
that created them is removed from the tree.

this prevents unwanted line breaks caused by the stray anonymous blocks.

CCBUG: 176681


 M  +57 -22    render_block.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=939175

SVN commit 939176 by ggarand:

Rephrase the line-breaking algorithm, so that inline flows are no longer
skipped and accounted for retroactively as part of text objects.

Inline flows have to stand on their own. They might have fancy
border/padding/margin, that can be different on each end.

We will still skip them eventually, but only after their endings
have been gauged.

Ditto when deciding if a given render object requires a line box or
can be skipped as part of initial whitespace.
you could have nested inline flows with different b/p/m at each end,
so both ends must be walked.

Initial inline flows will also trigger the ignoringSpaces state as needed -
indeed their b/p/m is not supposed to affect the treatment of initial
whitespace.

BUG: 176681


 M  +113 -143  bidi.cpp  
 M  +65 -0     bidi.h  
 M  +0 -68     render_block.cpp  
 M  +2 -2      render_line.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=939176

and just when I finally fix this bug, a javascript parsing error crops up on Slashdot that prevents the tag feature to work at all.. TANJ!


Error: http://images.slashdot.org/all-minified.js?T_2_5_0_246a: Parse error at <regexp> line 2
Error: http://slashdot.org/: TypeError: Undefined value

You can thank Acid3 for that --- based on a test in it, the RegExp parser was made a lot pickier. We're actually correct by the book here, as the standard regular expression grammar in ES3 does not permit ] as a standalone character,
so the / (.+)]$/ is permitted to raise an exception. Permitted because the spec also has some (idiotic) language that permits implementations to parse additional expressions, which makes that Acid3 test iffy, as what doing what it says is a failure is permitted behavior (though the grammar in ES3.1 drafts disallows what it tests, but not this). 

At any rate.. Well, the relevant code in pcre, the maintainer of which kindly added the mode for us. In this case, we can probably get the webmaster to fix it (if I can find someone with a contact, especially); but there is an another related report, about mootools or such. Even if that's fixed, there will likely be more... I'll talk to Harri about this, since he talked with the pcre dude.

P.S. would be nice if you backported the stuff quicker.

> You can thank Acid3 for that --- based on a test in it, the RegExp parser was
> made a lot pickier. We're actually correct by the book here, as the standard
> regular expression grammar in ES3 does not permit ] as a standalone character,
I see... Yet another ambiguity weighted upon by this test, with hardly any control from standard bodies.
When the test is actually more popular than the standards, what can you expect?

How do other engines cope with satisfying the test and having this
regexp pass? they have some compatibility mode?

> P.S. would be nice if you backported the stuff quicker.
yes, sure... I usually wait for some time to see if the code is stable before backporting - especially if the patch is meanie, but given there's still  10 days before next tagging...

Well, the acid3 tests is related, but different. This particular case was read off the spec (by me, actually, now that I think of it) when the PCRE author asked about it. Presumably FF changed behavior on what Acid3 tests, but didn't remove their extensions in other cases. Anyway, if you want to test w/o this:
--- regexp.cpp  (revision 936928)
+++ regexp.cpp  (working copy)
@@ -40,6 +40,8 @@
 using std::memset;
 using std::memcpy;

+#undef PCRE_JAVASCRIPT_COMPAT
+
 namespace KJS {

 RegExp::UTF8SupportState RegExp::utf8Support = RegExp::Unknown;


As for regressions: vtokarev mentioned something about a google reader problem.

And thanks for reminding about the deadline, I think we may want V.T.'s performance stuff for CSS in 4.2.2 (and I have some simple ideas for further DOM improvements, though some things that need saneifying in the tokenizer are likely to be too invasive)

So in other words, other engines just bothered to satisfy the RE test, and didn't even try to look further, while we get burnt for trying to do things consistently and properly... what a perfect world.

> Anyway, if you want to test w/o this:

yes it does work fine, sadly... I hope you can solve this with the maintainer - though this looks basically like asking for an intermediate mode or more fine grained features :-/
btw, acid3 test 90 is still failing for me with lib pcre 7.8 ("Test 90 failed: NUL in regexp didn't match correctly")?

> As for regressions: vtokarev mentioned something about a google reader problem.

regressions from r939175/r939176 or from older stuff?

> I think we may want V.T.'s performance stuff for CSS in 4.2.2

it looks great, yes. Very much welcome.

> and I have some simple ideas for further DOM improvements
yay for simple ideas with excellent effects! :)

Created attachment 32140 [details]
konq rendering of gg reader

Created attachment 32141 [details]
iceweasel rendering of gg reader

As of to gg reader regression, after r939175 in trunk I noted it. Text entries disappeared in the list of subscriptions, but you need to have a google account for that, sorry :(

@Viacheslav: thank you for the pointer (and btw, kudos for your great css && editing work! - hope the editing code won't drive you to madness :)

I probably missed a midpoint interaction - those can be real buggers.
reduction:

data:text/html,<div style="position: absolute"></div><span style="padding:10px">Lorem</span>

SVN commit 939854 by ggarand:

automatically merged revision 939175:
merge continuations back into a single inline when the block
that created them is removed from the tree.

this prevents unwanted line breaks caused by the stray anonymous blocks.

CCBUG: 176681

 M  +57 -22    render_block.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=939854

SVN commit 940706 by ggarand:

fix tricky regression noticed by Vyacheslav Tokarev on Google Reader.

the midpoint flip/flop machinery won't work properly when served skippable
InlineFlows, so have addMidpoint advance to the next object in that case.

also remove the adjustEmbedding global bool, now expressed by the
passing (or not) of a BidiState*.

CCBUG: 176681


 M  +49 -53    bidi.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=940706

SVN commit 940709 by ggarand:

automatically merged revision 939176:
Rephrase the line-breaking algorithm, so that inline flows are no longer
skipped and accounted for retroactively as part of text objects.

Inline flows have to stand on their own. They might have fancy
border/padding/margin, that can be different on each end.

We will still skip them eventually, but only after their endings
have been gauged.

Ditto when deciding if a given render object requires a line box or
can be skipped as part of initial whitespace.
you could have nested inline flows with different b/p/m at each end,
so both ends must be walked.

Initial inline flows will also trigger the ignoringSpaces state as needed -
indeed their b/p/m is not supposed to affect the treatment of initial
whitespace.

BUG: 176681

 M  +113 -143  bidi.cpp  
 M  +65 -0     bidi.h  
 M  +0 -68     render_block.cpp  
 M  +2 -2      render_line.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=940709

SVN commit 940712 by ggarand:

automatically merged revision 940706:
fix tricky regression noticed by Vyacheslav Tokarev on Google Reader.

the midpoint flip/flop machinery won't work properly when served skippable
InlineFlows, so have addMidpoint advance to the next object in that case.

also remove the adjustEmbedding global bool, now expressed by the
passing (or not) of a BidiState*.

CCBUG: 176681

 M  +49 -53    bidi.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=940712

@Maksim: would you find acceptable a patch that would detect this extension in the pattern (*after* a compilation error), then escape it and attempt a recompilation?

It seems to me if the extension is non-standard, we don't have to care about maximum performance and can afford having it failing the first time..

(alright, I do have the patch, so I'll attach it... if you don't like it, or have one of your own, just ignore, I don't care at all :)

Created attachment 32249 [details]
patch for regexp problem

SVN commit 942116 by ggarand:

my previous attempt at merging continuations back into inlines
in removeChild was way too shallow.

this ought to work for nested inlines too, so lets walk the chain
all the way back to the root anonymous block while merging.

this should cover most cases. To get a 100% correct behaviour, we will
have to get rid of continuations and use some sort of implicit inline
containers for hosting blocks in inlines.

CCBUG: 176681

 M  +41 -23    render_block.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=942116

SVN commit 942117 by ggarand:

More regression fixing in bidi hell..

Avoid testing for line break if the lBreak iterator contains an inlineflow.
InlineFlows are never a suitable line-breaking opportunity.

CCBUG: 176681

 M  +64 -62    bidi.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=942117