December 2009 - Dillo-dev - lists.mailman3.com

display: inline

by corvid＠lavabit.com

Johannes wrote: > Also I don't know, whether e.g. inputs in forms which have > DISPLAY_NONE, should send their contents to the server in case of > submit, or should they be just non-existent? I'm not aware of anything in html4 about that case. I think css2 says somewhere that it doesn't specify what to do with forms.

Dec. 9, 2009

display: inline

by tim.nieradzik＠gmx.de

> Just yesterday, I added the DISPLAY_NONE to dillo mainline, just as > you did here :-) Oh, I just had a look at your private repository and noticed that it wasn't yet implemented. :) > I'm still uncertain on what to do in case that DISPLAY_NONE is set. > As you noticed it's not enough to not call the tag open functions, > as text will still be added. I'd suggest to add the elements to the dw representation regardless of their "display" value. But we should introduce a 'flag' that disables drawing of elements with "display: none". > Also I don't know, whether e.g. inputs in forms which have > DISPLAY_NONE, should send their contents to the server in case of > submit, or should they be just non-existent? Since CSS only "describes the presentation semantics" (Wikipedia), the form elements are still available but 'hidden'. In Gecko, the DOM contains hidden elements as well. "display: none" is commonly used for menus: With the help of JavaScript, you could fiddle inside the DOM and toggle "display"-value to make the submenu visible. > Regarding tagIsDisplayEnabled, I don't quite follow what you > intended? I had added this flag to gradually transform the > Html_tag_open_*() functions to the new display handling. It > can only be removed once all these functions have been converted. I thought the only sense in implementing tagIsDisplayEnabled was to prevent Dillo to crash? While playing around with DISPLAY_NONE detection, the "stack->getRef (stack->size () - 1)->style == NULL" assert failed for Html_tag_open_body(). Checking html->styleEngine->style ()->display for DISPLAY_NONE resulted in initializing the style engine too early. > Hm, maybe you are right and as long as the corresponding tags are > set to DISPLAY_INLINE, which is the default, it would work even > without the tagIsDisplayEnabled flag. Anyway, I think it's still a > good marker to see which functions have been converted. To what extent would they need to be converted? Why do we care at all if a tag supports the "display" attribute? --Tim

Dec. 9, 2009

display: inline

by Johannes.Hofmann＠gmx.de

Hi Tim, On Wed, Dec 09, 2009 at 12:54:54AM +0100, Tim Nieradzik wrote: > In my opinion, these changes make sense indeed as they provide a more > generic interface. > > Looking at quirkmode's "display: list-item"-demo, listStylePosition > seems to have a wrong value. The bullets are supposed to be drawn > outside the box but they appear inside. Did I understand your changes > correctly that listStylePosition is either 0 ("outside") or 1 ("inside") > because of that enum? > > See http://www.quirksmode.org/css/display.html I guess you're right, that the bullets should be outside the box, but due to the current implementation of ListItem it's inside even if listStylePosition is outside. However as long as you don't look at the border of the box, the behaviour is correct now, see e.g. http://de.selfhtml.org/css/eigenschaften/anzeige/list_style_position.htm > > I tried to implement "display: none" but the contents are still visible. > I also worked on getting rid of tagIsDisplayEnabled but I am yet > uncertain whether it is safe how approached it. Any comments? Just yesterday, I added the DISPLAY_NONE to dillo mainline, just as you did here :-) I'm still uncertain on what to do in case that DISPLAY_NONE is set. As you noticed it's not enough to not call the tag open functions, as text will still be added. Also I don't know, whether e.g. inputs in forms which have DISPLAY_NONE, should send their contents to the server in case of submit, or should they be just non-existent? Regarding tagIsDisplayEnabled, I don't quite follow what you intended? I had added this flag to gradually transform the Html_tag_open_*() functions to the new display handling. It can only be removed once all these functions have been converted. Hm, maybe you are right and as long as the corresponding tags are set to DISPLAY_INLINE, which is the default, it would work even without the tagIsDisplayEnabled flag. Anyway, I think it's still a good marker to see which functions have been converted. Regards, Johannes > > --Tim > diff -r 440fa7e952bd dw/style.hh > --- a/dw/style.hh Tue Dec 08 18:46:31 2009 +0100 > +++ b/dw/style.hh Wed Dec 09 00:48:03 2009 +0100 > @@ -253,7 +253,8 @@ > DISPLAY_TABLE_FOOTER_GROUP, > DISPLAY_TABLE_ROW, > DISPLAY_TABLE_CELL, > - DISPLAY_LAST > + DISPLAY_LAST, > + DISPLAY_NONE > }; > > enum ListStylePosition { > diff -r 440fa7e952bd src/cssparser.cc > --- a/src/cssparser.cc Tue Dec 08 18:46:31 2009 +0100 > +++ b/src/cssparser.cc Wed Dec 09 00:48:03 2009 +0100 > @@ -58,10 +58,10 @@ > "w-resize", "text", "wait", "help", NULL > }; > > -static const char *const Css_display_enum_vals[DISPLAY_LAST + 1] = { > +static const char *const Css_display_enum_vals[] = { > "block", "inline", "list-item", "table", "table-row-group", > "table-header-group", "table-footer-group", "table-row", > - "table-cell", NULL > + "table-cell", "last", "none", NULL > }; > > static const char *const Css_font_size_enum_vals[] = { > diff -r 440fa7e952bd src/html.cc > --- a/src/html.cc Tue Dec 08 18:46:31 2009 +0100 > +++ b/src/html.cc Wed Dec 09 00:48:03 2009 +0100 > @@ -1282,7 +1282,7 @@ > TagInfo toptag = Tags[toptag_idx]; > if (s_sz > idx + 1 && toptag.EndTag != 'O') > BUG_MSG(" - forcing close of open tag: <%s>\n", toptag.name); > - _MSG("Close: %*s%s\n", size," ", toptag.name); > + _MSG("Close: %*s%s\n", s_sz, " ", toptag.name); > toptag.close(html, toptag_idx); > Html_real_pop_tag(html); > } > @@ -1528,13 +1528,6 @@ > } > > /* > - * By setting this to true, a Html_tag_open* function indicates that > - * the common display handling code should be used. > - * TODO: convert all Html_tag_open* functions and remove this variable. > - */ > -static bool tagIsDisplayEnabled; > - > -/* > * Handle open HTML element > */ > static void Html_tag_open_html(DilloHtml *html, const char *tag, int tagsize) > @@ -1876,8 +1869,6 @@ > a_Html_stash_init(html); > S_TOP(html)->parse_mode = > DILLO_HTML_PARSE_MODE_STASH_AND_BODY; > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -1916,8 +1907,6 @@ > > html->styleEngine->setNonCssHints (&props); > dFree(fontFamily); > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -1926,7 +1915,7 @@ > static void Html_tag_open_abbr(DilloHtml *html, const char *tag, int tagsize) > { > const char *attrbuf; > - > + > if (prefs.show_tooltip && > (attrbuf = a_Html_get_attr(html, tag, tagsize, "title"))) { > CssPropertyList props; > @@ -1936,8 +1925,6 @@ > html->styleEngine->setNonCssHints (&props); > dFree(tooltip_str); > } > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -2183,8 +2170,6 @@ > BUG_MSG("name attribute is required for <map>\n"); > } > } > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -2453,8 +2438,6 @@ > dFree(nameVal); > } > } > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -2528,8 +2511,6 @@ > S_TOP(html)->list_type = HTML_LIST_UNORDERED; > S_TOP(html)->list_number = 0; > S_TOP(html)->ref_list_item = NULL; > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -2590,10 +2571,9 @@ > BUG_MSG( "illegal '-' character in START attribute; Starting from 0\n"); > n = 0; > } > + > S_TOP(html)->list_number = n; > S_TOP(html)->ref_list_item = NULL; > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -2621,8 +2601,6 @@ > *list_number = 0; > } > } > - > - tagIsDisplayEnabled = true; > } > > /* > @@ -3011,7 +2989,6 @@ > static void Html_tag_open_default(DilloHtml *html,const char *tag,int tagsize) > { > html->styleEngine->inheritBackgroundColor(); > - tagIsDisplayEnabled = true; > } > > /* > @@ -3023,7 +3000,6 @@ > > a_Html_tag_set_align_attr (html, &props, tag, tagsize); > html->styleEngine->setNonCssHints (&props); > - tagIsDisplayEnabled = true; > } > > /* > @@ -3418,11 +3394,11 @@ > ListItem *list_item; > int *list_number; > char buf[16]; > - > + > /* Get our parent tag's variables (used as state storage) */ > list_number = &html->stack->getRef(html->stack->size()-2)->list_number; > ref_list_item = &html->stack->getRef(html->stack->size()-2)->ref_list_item; > - > + > HT2TB(html)->addParbreak (0, wordStyle); > > list_item = new ListItem ((ListItem*)*ref_list_item,prefs.limit_text_width); > @@ -3430,7 +3406,7 @@ > HT2TB(html)->addParbreak (0, wordStyle); > *ref_list_item = list_item; > S_TOP(html)->textblock = html->dw = list_item; > - > + > if (style->listStyleType == LIST_STYLE_TYPE_NONE) { > // none > } else if (style->listStyleType >= LIST_STYLE_TYPE_DECIMAL) { > @@ -3496,15 +3472,11 @@ > /* Parse attributes that can appear on any tag */ > Html_parse_common_attrs(html, tag, tagsize); > > - /* Html_tag_open* function can set this to true to enable > - * generic CSS display handling. > - */ > - tagIsDisplayEnabled = false; > - /* Call the open function for this tag */ > - _MSG("Open : %s\n", Tags[ni].name); > - Tags[ni].open (html, tag, tagsize); > - > - if (tagIsDisplayEnabled) { > + if (! html->styleEngine->initialized () > + || html->styleEngine->style ()->display == DISPLAY_NONE) { > + /* Call the open function for this tag */ > + Tags[ni].open (html, tag, tagsize); > + > switch (html->styleEngine->style ()->display) { > case DISPLAY_BLOCK: > Html_display_block(html); > @@ -3549,7 +3521,6 @@ > (strchr(" \"'", tag[tagsize-3]) || /* [ "']/> */ > (size_t)tagsize == strlen(Tags[ni].name) + 3))) { /* <x/> */ > > - _MSG("Close: %s\n", Tags[ni].name); > Html_tag_cleanup_at_close(html, ni); > /* This was a close tag */ > html->ReqTagClose = false; > diff -r 440fa7e952bd src/styleengine.cc > --- a/src/styleengine.cc Tue Dec 08 18:46:31 2009 +0100 > +++ b/src/styleengine.cc Wed Dec 09 00:48:03 2009 +0100 > @@ -567,6 +567,10 @@ > return Style::create (layout, &attrs); > } > > +bool StyleEngine::initialized () { > + return stack->getRef (stack->size () - 1)->style != NULL; > +} > + > /** > * \brief Create a new style object based on the previously opened / closed > * HTML elements and the nonCssProperties that have been set. > @@ -585,7 +589,7 @@ > // If this assertion is hit, you need to rearrange the code that is > // doing styleEngine calls to call setNonCssHints() before calling > // style() or wordStyle() for each new element. > - assert (stack->getRef (stack->size () - 1)->style == NULL); > + assert (! StyleEngine::initialized()); > > // reset values that are not inherited according to CSS > attrs.resetValues (); > diff -r 440fa7e952bd src/styleengine.hh > --- a/src/styleengine.hh Tue Dec 08 18:46:31 2009 +0100 > +++ b/src/styleengine.hh Wed Dec 09 00:48:03 2009 +0100 > @@ -59,6 +59,7 @@ > return NULL; > }; > > + bool initialized (); > void parse (DilloHtml *html, DilloUrl *url, const char *buf, int buflen, > CssOrigin origin); > void startElement (int tag);

Dec. 9, 2009

display: inline

by tim.nieradzik＠gmx.de

In my opinion, these changes make sense indeed as they provide a more generic interface. Looking at quirkmode's "display: list-item"-demo, listStylePosition seems to have a wrong value. The bullets are supposed to be drawn outside the box but they appear inside. Did I understand your changes correctly that listStylePosition is either 0 ("outside") or 1 ("inside") because of that enum? See http://www.quirksmode.org/css/display.html I tried to implement "display: none" but the contents are still visible. I also worked on getting rid of tagIsDisplayEnabled but I am yet uncertain whether it is safe how approached it. Any comments? --Tim

Dec. 9, 2009

display: inline

by Johannes.Hofmann＠gmx.de

On Tue, Dec 08, 2009 at 08:36:35PM +0000, corvid wrote: > Johannes wrote: > > Please let me know what you think, and I would especially like to > > get opinions on how to implement the various display types: > > > > inline | block | list-item | run-in | inline-block | none > > I don't know about details, but I suspect the logic will all > end up in dw. I thought that the display handling would be done in html.cc - by choosing the proper dw widget structure. I think this is what Sebastian calls "complex properties" in http://www.dillo.org/CSS.html > > And that we will end up having to make a lot of boxes eventually. Yes definately. Currently we sometimes just add a parbreak, where we would need a new box. That's also the reason for the inheritBackgroundColor hack. On the other hand more Textblock widgets consume more memory on large pages. > In bug 929, someone wants to know why padding-left doesn't work > for P elements, for instance. This might be what Sebastian meant in > the top of textblock.hh about collapsing spaces being unnecessary > in a CSS-ized dillo. I don't know enough about this space collapsing...

Dec. 8, 2009

display: inline

by corvid＠lavabit.com

Johannes wrote: > Please let me know what you think, and I would especially like to > get opinions on how to implement the various display types: > > inline | block | list-item | run-in | inline-block | none I don't know about details, but I suspect the logic will all end up in dw. And that we will end up having to make a lot of boxes eventually. In bug 929, someone wants to know why padding-left doesn't work for P elements, for instance. This might be what Sebastian meant in the top of textblock.hh about collapsing spaces being unnecessary in a CSS-ized dillo.

Dec. 8, 2009

display: inline

by Johannes.Hofmann＠gmx.de

Hi, I started to experiment with the real solution, which means that the display member of Style determines whether a (Text)block is created or a ListItem, or whether just normal text is added. The patch is still small, but as the change is pretty fundamental, so I decided to put it in a separate repo for now. You can find it at: http://www.flpsed.org/hgweb/dillo_display/ Please let me know what you think, and I would especially like to get opinions on how to implement the various display types: inline | block | list-item | run-in | inline-block | none I've intentionally left out the table stuff for now ... Cheers, Johannes

Dec. 8, 2009

display: inline

by tim.nieradzik＠gmx.de

Hi Johannes, thank you very much. The patch works great for <li> elements but for each <ul> there is still some unused space. See http://www.heise.de/. I added "display: list-item" to the ul-item in Dillo's stylesheet definition and in Html_tag_open_ul() I am comparing the the display-attribute with the "list-item"-value. Strangely enough it worked! I am still wondering why it does. --Tim

Dec. 6, 2009

display: inline

by Johannes.Hofmann＠gmx.de

Hi Tim, On Sat, Dec 05, 2009 at 09:10:45PM +0100, Tim Nieradzik wrote: > I am currently working on adding CSS support for "display: inline" tags. > They are commonly used for navigation menus. As these menus are nothing > more than simple unordered lists (<ul>), the items are printed one below > the other which wastes lots of space. > > The patch I came up with does only partially implement support for these > tags: The items are correctly displayed within one line but I cannot > figure out why there is so much empty space after each item. > > Perhaps somebody with a better understanding of the dw-code than me > might have a look into the issue. If I am correct, the dw/textblock.cc > is responsible for calculating the widths. Textblock::wordWrap() looks > pretty promising as there is a variable (hasListitemValue) determining > whether it is a list item and calculates values for lastLine->leftOffset > and words->getRef(0)->effSpace depending on this state. I think we need to be a bit more radical. Consider this slightly modified patch (attached). If I read the CSS 2.1 Spec correctly then the idea is that every element can be a list-item if it has display: list-item set. So <li> elements would have to be handled as any other element but get display: list-item from our user-agent style. So for the real implementation we would need to change html.cc in a way that not the current element name, but the value of the display property determines whether to show a list item, or to show nothing at all (display: none) and so on. That said, I think the modified version of your patch is a good start until we fully support the display property, as lists with display: inline seem to be pretty common. So please test this with your favorite web sites. Cheers, Johannes

Dec. 6, 2009

HTML parser bug

by jcid＠dillo.org

On Sat, Dec 05, 2009 at 08:47:56PM +0000, corvid wrote: > Tim wrote: > > There is a bug in the HTML parser: Tags within quotes are interpreted: > > > > <input type="text" name="test" value="<p>asdf</p>" /> > > > > I think it is line 3754f (src/html.cc) which evokes the unwanted > > behaviour. I'd fix this bug by introducing two variables: The first one > > states whether we're currently inside of a quote and the second one > > stores its type (single or double quote). As long as the current > > character does not equal the type, all characters in between the > > starting and ending quote will be ignored. Of course we should be also > > able to deal with escaped quotes allowing constructs similar to the > > following one: > > > > <input type="text" name="test" value="<p>\"asdf\"</p>" /> > > Strictly speaking, for attributes that are CDATA, at least, > my impression is that the element is closed if the parser encounters an > (SGML jargon!) end-tag open (ETAGO, i.e., "</") "delimiter-in-context" > (meaning, I think, that it is followed by certain characters such as > ASCII letters). > > Of course, sgml is a horrible, loony monstrosity, and we don't follow > it to the letter to begin with, so I'm not sure that I'd necessarily be > opposed to making the parser behave as you say. > (Remembers http://lists.auriga.wearlab.de/pipermail/dillo-dev/2008-January/003668.html) Good advice! ;) Although a better heuristic is welcomed, it's not easy. Beware of the sleeping dragon. You have to consider (at least): * RFC (HTML, SGML, XML) * What do other browsers do. * Heuristic worst case and average. * Unexpected input (broken HTML or XML, attacks...). * Complexity and overhead (it affects the tokenizer). * Error reporting. The above idea has been tried, but it fails when the attribute lacks its closing quote, sometimes eating lots of content when plain text follows. You can find more details in doc/HtmlParser.txt. I wish we could find a better heuristic. For instance to clean the IFRAME rendering of http://www.linuxfordevices.com/. -- Cheers Jorge.-

Dec. 5, 2009