[bug]: CSS image crash
If you point dillo at http://www.youtube.com/user/zynaddsubfx and scroll down then Dillo will crash before you get to the bottom. I have attached a minimal test case that reproduces this (strangely, the empty SRC attribute on the img element is necessary). GDB shows that the drawing code is dereferencing a NULL FltkColor*: #0 0x080a4811 in dw::fltk::FltkViewBase::drawPolygon (this=0x49f4318, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=true, points=0xbeae1a48, npoints=4) at /home/jeremy/.packages/dillo/HG/local/dw/fltkviewbase.cc:464 464 setcolor(((FltkColor*)color)->colors[shading]); Regards, Jeremy Henty
On Mon, Mar 21, 2011 at 10:30:33PM +0000, Jeremy Henty wrote:
If you point dillo at http://www.youtube.com/user/zynaddsubfx and scroll down then Dillo will crash before you get to the bottom. I have attached a minimal test case that reproduces this (strangely, the empty SRC attribute on the img element is necessary).
GDB shows that the drawing code is dereferencing a NULL FltkColor*:
#0 0x080a4811 in dw::fltk::FltkViewBase::drawPolygon (this=0x49f4318, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=true, points=0xbeae1a48, npoints=4) at /home/jeremy/.packages/dillo/HG/local/dw/fltkviewbase.cc:464 464 setcolor(((FltkColor*)color)->colors[shading]);
Regards,
I see a segfault using fltk-1.3 as well here when viewing the above URL. Although Dillo doesn't crash immediately when loading the page here, seems to happen after scrolling all the way down the page and shortly after starting to scroll upward. -- Roger http://rogerx.freeshell.org/
Roger wrote:
On Mon, Mar 21, 2011 at 10:30:33PM +0000, Jeremy Henty wrote:
If you point dillo at http://www.youtube.com/user/zynaddsubfx and scroll down then Dillo will crash before you get to the bottom. I have attached a minimal test case that reproduces this (strangely, the empty SRC attribute on the img element is necessary).
GDB shows that the drawing code is dereferencing a NULL FltkColor*:
#0 0x080a4811 in dw::fltk::FltkViewBase::drawPolygon (this=0x49f4318, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=true, points=0xbeae1a48, npoints=4) at /home/jeremy/.packages/dillo/HG/local/dw/fltkviewbase.cc:464 464 setcolor(((FltkColor*)color)->colors[shading]);
I see a segfault using fltk-1.3 as well here when viewing the above URL.
Interesting. What happens when you point the fltk-1.3 version at the test case I attached? I get an immediate crash with the fltk-2 version.
Although Dillo doesn't crash immediately when loading the page here, seems to happen after scrolling all the way down the page and shortly after starting to scroll upward.
I think the crash happens only when the image with the offending style is actually displayed, which is why you have to scroll down the original page. It looks as though the bug is due to the style engine assigning a NULL color somewhere. Your backtrace shows the same thing as mine - FltkViewBase::drawPolygon() is called with a NULL color argument. This suggests that fixing the style engine will fix the crash for both versions. Who understands the style engine? Regards, Jeremy Henty
On Mon, Mar 21, 2011 at 10:30:33PM +0000, Jeremy Henty wrote:
If you point dillo at http://www.youtube.com/user/zynaddsubfx and scroll down then Dillo will crash before you get to the bottom. I have attached a minimal test case that reproduces this (strangely, the empty SRC attribute on the img element is necessary).
GDB shows that the drawing code is dereferencing a NULL FltkColor*:
#0 0x080a4811 in dw::fltk::FltkViewBase::drawPolygon (this=0x49f4318, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=true, points=0xbeae1a48, npoints=4) at /home/jeremy/.packages/dillo/HG/local/dw/fltkviewbase.cc:464 464 setcolor(((FltkColor*)color)->colors[shading]);
Regards,
Jeremy Henty
For what it's worth, here's my GDB session. --- Begin Snip --- Nav_open_url: new url='http://www.youtube.com/user/zynaddsubfx' a_Nav_expect_done: repush! Html_tag_open_link(): addCssUrl http://s.ytimg.com/yt/cssbin/www-core-vfll8nFx1.css Html_tag_open_link(): addCssUrl http://s.ytimg.com/yt/cssbin/www-the-rest-vflgCq-F2.css Html_tag_open_link(): addCssUrl http://s.ytimg.com/yt/cssbin/www-channel_new-vflebJ8FK.css no values for shorthand property 'border-color' no values for shorthand property 'border-color' no values for shorthand property 'border-color' DNS error: HOST_NOT_FOUND Dns_server [1]: i2.ytimg.com is (nil) [Thread 0xb7c2bb70 (LWP 5397) exited] Program received signal SIGSEGV, Segmentation fault. _______________________________________________________________________________ eax:441D7F20 ebx:00000004 ecx:00000000 edx:00000000 eflags:00010202 esi:0810D268 edi:BFFFDA00 esp:BFFFD930 ebp:BFFFD978 eip:080987F9 cs:0073 ds:007B es:007B fs:0000 gs:0033 ss:007B o d I t s z a p c [007B:BFFFD930]---------------------------------------------------------[stack] BFFFD960 : 60 4A 38 B7 60 59 2F 08 - E8 D9 FF BF 78 44 2F 08 `J8.`Y/.....xD/. BFFFD950 : 51 00 01 01 04 00 00 00 - 98 D9 FF BF 78 4D 2F 08 Q...........xM/. BFFFD940 : 40 AF 10 08 00 03 00 00 - C4 3B 14 08 50 D9 FF BF @........;..P... BFFFD930 : 94 E9 FF BF 60 57 10 08 - 58 D9 FF BF AF 4E D4 46 ....`W..X....N.F [007B:0810D268]---------------------------------------------------------[ data] 0810D268 : 28 2E 0C 08 E0 2E 0C 08 - 70 09 10 08 50 E8 15 44 (.......p...P..D 0810D278 : 00 00 00 00 00 00 00 00 - 75 00 00 00 FE 04 00 00 ........u....... [0073:080987F9]---------------------------------------------------------[ code] => 0x80987f9 <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+57>: mov 0xc(%edx,%ecx,4),%edx 0x80987fd <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+61>: mov (%eax),%ebx 0x80987ff <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+63>: mov %edx,0x4(%esp) 0x8098803 <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+67>: mov %eax,(%esp) 0x8098806 <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+70>: call *0x3c(%ebx) 0x8098809 <dw::fltk::FltkViewBase::drawPolygon(dw::core::style::Color*, dw::core::style::Color::Shading, bool, bool, int (*) [2], int)+73>: cmpb $0x0,-0x25(%ebp) ------------------------------------------------------------------------------ 0x080987f9 in fl_color (this=0x810d268, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=0x1, convex=0x1, points=0xbfffda00, npoints=0x4) at /usr/include/fltk-1.1/FL/fl_draw.H:61 61 inline void fl_color(Fl_Color c) {fl_graphics_driver->color(c); } // select indexed color gdb> bt #0 0x080987f9 in fl_color (this=0x810d268, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=0x1, convex=0x1, points=0xbfffda00, npoints=0x4) at /usr/include/fltk-1.1/FL/fl_draw.H:61 #1 fl_color (this=0x810d268, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=0x1, convex=0x1, points=0xbfffda00, npoints=0x4) at /usr/include/fltk-1.1/FL/fl_draw.H:63 #2 dw::fltk::FltkViewBase::drawPolygon (this=0x810d268, color=0x0, shading=dw::core::style::Color::SHADING_NORMAL, filled=0x1, convex=0x1, points=0xbfffda00, npoints=0x4) at fltkviewbase.cc:497 #3 0x080a4587 in drawBorderBottom (view=0x810d268, area=0xbfffda6c, x=0x7, y=0x456, width=0x2e, height=0x9, style=0x82f4478, inverse=0x0) at style.cc:559 #4 dw::core::style::drawBorder (view=0x810d268, area=0xbfffda6c, x=0x7, y=0x456, width=0x2e, height=0x9, style=0x82f4478, inverse=0x0) at style.cc:840 #5 0x080a7518 in dw::core::Widget::drawWidgetBox (this=0x82f42c0, view=0x810d268, area=0xbfffdb6c, inverse=0x0) at widget.cc:385 #6 0x0808efca in dw::Textblock::draw (this=0x82f42c0, view=0x810d268, area=0xbfffdb6c) at textblock.cc:1534 #7 0x0808d7f6 in dw::Textblock::drawLine (this=0x82f4120, line=0x82fb260, view=0x810d268, area=0xbfffdc0c) at textblock.cc:1416 #8 0x0808f09c in dw::Textblock::draw (this=0x82f4120, view=0x810d268, area=0xbfffdc0c) at textblock.cc:1543 #9 0x08089fab in dw::Table::draw (this=0xb7330d20, view=0x810d268, area=0xbfffdcbc) at table.cc:256 #10 0x0808d7f6 in dw::Textblock::drawLine (this=0xb730b618, line=0xb7331260, view=0x810d268, area=0xbfffdd5c) at textblock.cc:1416 #11 0x0808f09c in dw::Textblock::draw (this=0xb730b618, view=0x810d268, area=0xbfffdd5c) at textblock.cc:1543 #12 0x08089fab in dw::Table::draw (this=0xb7330960, view=0x810d268, area=0xbfffde0c) at table.cc:256 #13 0x0808d7f6 in dw::Textblock::drawLine (this=0xb730b848, line=0xb73d9db8, view=0x810d268, area=0xbfffdf0c) at textblock.cc:1416 #14 0x0808f09c in dw::Textblock::draw (this=0xb730b848, view=0x810d268, area=0xbfffdf0c) at textblock.cc:1543 #15 0x0808d7f6 in dw::Textblock::drawLine (this=0xb730bbd8, line=0xb734db58, view=0x810d268, area=0xbfffe00c) at textblock.cc:1416 #16 0x0808f09c in dw::Textblock::draw (this=0xb730bbd8, view=0x810d268, area=0xbfffe00c) at textblock.cc:1543 #17 0x0808d7f6 in dw::Textblock::drawLine (this=0xb730ba30, line=0xb73e60c8, view=0x810d268, area=0xbfffe10c) at textblock.cc:1416 #18 0x0808f09c in dw::Textblock::draw (this=0xb730ba30, view=0x810d268, area=0xbfffe10c) at textblock.cc:1543 #19 0x0808d7f6 in dw::Textblock::drawLine (this=0x82fa700, line=0x82faa3c, view=0x810d268, area=0xbfffe20c) at textblock.cc:1416 #20 0x0808f09c in dw::Textblock::draw (this=0x82fa700, view=0x810d268, area=0xbfffe20c) at textblock.cc:1543 --- End Snip --- -- Roger http://rogerx.freeshell.org/
Jeremy Henty wrote:
If you point dillo at http://www.youtube.com/user/zynaddsubfx and scroll down then Dillo will crash before you get to the bottom. [...]
GDB shows that the drawing code is dereferencing a NULL FltkColor*:
OK, StyleEngine::apply() in styleengine.cc sets border colors to NULL when the corresponding CSS property is transparent. Which seems OK. These are used by static functions drawBorder{Top,Left,Bottom,Right}() in dw/style.cc . These functions are clearly intended to handle NULL colors, eg. drawBorderTop() checks whether style->borderColor.top is NULL, and similarly for the others. The bug is that some of these functions may (depending on style->borderStyle) dereference other members of style->borderColor without checking whether they are NULL. In particular: *) drawBorderBottom() may reference style->borderColor.top *) drawBorderLeft() may reference style->borderColor.top So I guess the fix is to add extra guards to the code paths where other members of style->borderColor are referenced. I am puzzled by the lack of symmetry between these functions. Why do drawBorder{Bottom,Left}() sometimes use style->borderColor.top, yet drawBorder{Top,Right} only ever use their *own* color? (This is all specific to FLTK-2.0 .) Comments? Regards, Jeremy Henty
OK, patch attached. Please comment. I just added checks for a NULL borderColor.top where necessary. I am still not sure if the drawBorder* functions are doing exactly the right thing but at the least the patch stops the crash and I don't think it makes the behaviour any worse. Please comment, Regards, Jeremy Henty
On Tue, Mar 22, 2011 at 04:04:06PM +0000, Jeremy Henty wrote:
OK, patch attached. Please comment. I just added checks for a NULL borderColor.top where necessary. I am still not sure if the drawBorder* functions are doing exactly the right thing but at the least the patch stops the crash and I don't think it makes the behaviour any worse.
Please comment,
To me the references to style->borderColor.top in drawBorder*() other than drawBorderTop() look like a typo, but I might be wrong. Jorge, can you please have a look? Cheers, Johannes
Johannes Hofmann wrote:
To me the references to style->borderColor.top in drawBorder*() other than drawBorderTop() look like a typo,
I thought so too at first but it's less simple than I realised. The challenge is rendering borders in fancy styles. Check out the examples[1]. It is not obvious to me how to choose the right colors. The CSS spec does not help[2] - it leaves the choice of colors up to the user agent. It gives informal descriptions of the border styles but they really just say "it must look how it should obviously look". Regards, Jeremy Henty [1] http://www.dillo.org/css_compat/tests/border-style.html [2] http://www.w3.org/TR/CSS2/box.html - search for "8.5.3 Border style:"
On Wed, Mar 23, 2011 at 10:10:35PM +0000, Jeremy Henty wrote:
Johannes Hofmann wrote:
To me the references to style->borderColor.top in drawBorder*() other than drawBorderTop() look like a typo,
I thought so too at first but it's less simple than I realised. The challenge is rendering borders in fancy styles. Check out the examples[1]. It is not obvious to me how to choose the right colors.
The CSS spec does not help[2] - it leaves the choice of colors up to the user agent. It gives informal descriptions of the border styles but they really just say "it must look how it should obviously look".
Yeah, I also remember something like that, but I made a test case (http://www.dillo.org/css_compat/tests/border-color.html) and it looks wrong when compared e.g. with firefox, so I guess it's a typo. Regards, Johannes
Johannes Hofmann wrote:
... I made a test case (http://www.dillo.org/css_compat/tests/border-color.html) and it looks wrong when compared e.g. with firefox, so I guess it's a typo.
That is very convincing. I find that changing the drawBorderFoo() functions to use only the foo color fixes the crash *and* gives the same results as Firefox. <charlie_sheen>BI-WINNING!!!</charlie_sheen> I withdraw the previous patch and attach a new one. Thanks for the test case. Regards, Jeremy Henty
On Wed, Mar 23, 2011 at 08:56:53PM +0100, Johannes Hofmann wrote:
On Tue, Mar 22, 2011 at 04:04:06PM +0000, Jeremy Henty wrote:
OK, patch attached. Please comment. I just added checks for a NULL borderColor.top where necessary. I am still not sure if the drawBorder* functions are doing exactly the right thing but at the least the patch stops the crash and I don't think it makes the behaviour any worse.
Please comment,
To me the references to style->borderColor.top in drawBorder*() other than drawBorderTop() look like a typo, but I might be wrong. Jorge, can you please have a look?
I'm checking it now... -- Cheers Jorge.-
On Wed, Mar 23, 2011 at 08:56:53PM +0100, Johannes Hofmann wrote:
On Tue, Mar 22, 2011 at 04:04:06PM +0000, Jeremy Henty wrote:
OK, patch attached. Please comment. I just added checks for a NULL borderColor.top where necessary. I am still not sure if the drawBorder* functions are doing exactly the right thing but at the least the patch stops the crash and I don't think it makes the behaviour any worse.
Please comment,
To me the references to style->borderColor.top in drawBorder*() other than drawBorderTop() look like a typo, but I might be wrong. Jorge, can you please have a look?
Patch committed (just typos). I took some time to review it because borders are not simple. There's pixel matching, the case of simulated "collapsing" mode with asymmetric tricks, etc. Funny how we all were looking at it at the same time. :-) Attached goes an example that shows the corrected flaws. -- Cheers Jorge.-
Jorge Arellano Cid wrote:
On Fri, Mar 25, 2011 at 06:12:48PM +0000, Jeremy Henty wrote:
Jorge Arellano Cid wrote:
Patch committed (just typos).
Is it pushed? I just did a pull and got nothing newer than a commit from corvid on Feb 11th.
Yes, in the 1.3 branch.
So how do we fix 2.0? With my latest patch? Or should we port yours from 1.3 to 2.0? Regards, Jeremy Henty
On Sat, Mar 26, 2011 at 09:30:37AM +0000, Jeremy Henty wrote:
Jorge Arellano Cid wrote:
On Fri, Mar 25, 2011 at 06:12:48PM +0000, Jeremy Henty wrote:
Jorge Arellano Cid wrote:
Patch committed (just typos).
Is it pushed? I just did a pull and got nothing newer than a commit from corvid on Feb 11th.
Yes, in the 1.3 branch.
So how do we fix 2.0? With my latest patch? Or should we port yours from 1.3 to 2.0?
From a quick review they're the same. The one you posted surely applies cleanly to 2.0. Just commit it. -- Cheers Jorge.-
Jorge Arellano Cid wrote:
On Sat, Mar 26, 2011 at 09:30:37AM +0000, Jeremy Henty wrote:
So how do we fix 2.0? With my latest patch? Or should we port yours from 1.3 to 2.0?
From a quick review they're the same. The one you posted surely applies cleanly to 2.0. Just commit it.
OK, it's done. Regards, Jeremy Henty
On Sun, Mar 27, 2011 at 10:30:10AM +0100, Jeremy Henty wrote:
Jorge Arellano Cid wrote:
On Sat, Mar 26, 2011 at 09:30:37AM +0000, Jeremy Henty wrote:
So how do we fix 2.0? With my latest patch? Or should we port yours from 1.3 to 2.0?
From a quick review they're the same. The one you posted surely applies cleanly to 2.0. Just commit it.
OK, it's done.
Just for the record, normally it's better to make general changes and bugfixes to the main branch and merge them into development branches. But for simple stuff like this it doesn't really matter. Cheers, Johannes
Johannes Hofmann wrote:
Just for the record, normally it's better to make general changes and bugfixes to the main branch and merge them into development branches.
"main" being FLTK 2.0 and "development" being FLTK 1.3, is that right? Who is responsible for merging changes into development? So far I have been thinking only about the 2.0 branch and treating 1.3 as someone else's problem. Is that fair or should I be checking that my changes merge into the 1.3 bramnch too? Regards, Jeremy Henty
On Sun, Mar 27, 2011 at 03:56:27PM +0100, Jeremy Henty wrote:
Johannes Hofmann wrote:
Just for the record, normally it's better to make general changes and bugfixes to the main branch and merge them into development branches.
"main" being FLTK 2.0 and "development" being FLTK 1.3, is that right?
right
Who is responsible for merging changes into development? So far I have been thinking only about the 2.0 branch and treating 1.3 as someone else's problem. Is that fair or should I be checking that my changes merge into the 1.3 bramnch too?
No, that's fine. Merging into development branches should be done by the developer who feels responsible for that branch. Cheers, Johannes
participants (4)
-
jcid＠dillo.org -
Johannes.Hofmann＠gmx.de -
onepoint＠starurchin.org -
rogerx.oss＠gmail.com