Support for title attribute in tooltips
You are all likely to cringe in horror when you see what I have been up to today. I basically wanted to add hover/tooltip functionality for elements which contain the title attribute. I think I achieved that with the following code: diff -r 49c3e24bf746 dpi/downloads.cc --- a/dpi/downloads.cc Fri Jul 29 20:19:08 2011 -0400 +++ b/dpi/downloads.cc Sat Jul 30 18:58:38 2011 -0400 @@ -516,7 +516,7 @@ // FSM to remove wget's "dot-progress" (i.e. "^ " || "^[0-9]+K") q = log_text + log_len; - for (p = esc_str; (p - esc_str) < esc_len; ++p) { + for (p = esc_str; (size_t)(p - esc_str) < esc_len; ++p) { switch (log_state) { case ST_newline: if (*p == ' ') { diff -r 49c3e24bf746 src/html.cc --- a/src/html.cc Fri Jul 29 20:19:08 2011 -0400 +++ b/src/html.cc Sat Jul 30 18:58:38 2011 -0400 @@ -1788,7 +1788,6 @@ html->styleEngine->backgroundStyle()->backgroundColor->getColor()); } - S_TOP(html)->parse_mode = DILLO_HTML_PARSE_MODE_BODY; } @@ -3455,6 +3454,13 @@ html->styleEngine->setStyle (attrbuf); } + if (prefs.show_tooltip && + (attrbuf = a_Html_get_attr(html, tag, tagsize, "title"))) { + + html->styleEngine->setNonCssHint (PROPERTY_X_TOOLTIP, CSS_TYPE_STRING, + attrbuf); + } + } /* The real ugliness is here: http://www3.sympatico.ca/rsquared/lib/dillo/title.html :P I am not done yet, and I have some ideas of some other little things I can do. But first I want to check with you fellows if you think this is on the right track. I hate to do a bunch of code that works, but doesn't "work" for the rest of you for whatever reason. Regards, Rob
Rob wrote:
You are all likely to cringe in horror when you see what I have been up to today. I basically wanted to add hover/tooltip functionality for elements which contain the title attribute.
I remember discussing this with Johannes, and I think he had some reason that he was reluctant to check for the title attr there...? I hope he remembers because I can't seem to find record of it. BTW, strictly speaking, we shouldn't be calling Html_parse_common_attrs() for every element because I believe there were a few that don't have %coreattrs under ATTLIST in the spec.
I remember discussing this with Johannes, and I think he had some reason that he was reluctant to check for the title attr there...? I hope he remembers because I can't seem to find record of it.
BTW, strictly speaking, we shouldn't be calling Html_parse_common_attrs() for every element because I believe there were a few that don't have %coreattrs under ATTLIST in the spec.
Of course you are right. I was too eager to send something in, and was really swimming over my head. Going back to Html_parse_common_attrs(), it is plain that this is for all elements having common attributes, whereas there are a handful of elements that do not allow for the title attribute, and are therefore excluded. I'll go back to learning how this thing works, and hopefully Johannes will have something to add about the conversation you mentioned in the meantime. Regards, Rob
Rob wrote:
I remember discussing this with Johannes, and I think he had some reason that he was reluctant to check for the title attr there...? I hope he remembers because I can't seem to find record of it.
BTW, strictly speaking, we shouldn't be calling Html_parse_common_attrs() for every element because I believe there were a few that don't have %coreattrs under ATTLIST in the spec.
Of course you are right. I was too eager to send something in, and was really swimming over my head. Going back to Html_parse_common_attrs(), it is plain that this is for all elements having common attributes, whereas there are a handful of elements that do not allow for the title attribute, and are therefore excluded. I'll go back to learning how this thing works, and hopefully Johannes will have something to add about the conversation you mentioned in the meantime.
Well, it's equally true of the attrs already in there -- id, class, and style, so it's not necessarily a bad place. The declaration or whatever goes: <!ENTITY % coreattrs "id ID #IMPLIED -- document-wide unique id -- class CDATA #IMPLIED -- space-separated list of classes -- style %StyleSheet; #IMPLIED -- associated style info -- title %Text; #IMPLIED -- advisory title --"
On Sat, Jul 30, 2011 at 11:38:17PM +0000, corvid wrote:
Rob wrote:
You are all likely to cringe in horror when you see what I have been up to today. I basically wanted to add hover/tooltip functionality for elements which contain the title attribute.
I remember discussing this with Johannes, and I think he had some reason that he was reluctant to check for the title attr there...? I hope he remembers because I can't seem to find record of it.
I guess you are refering to this discussion: On Wed, Sep 30, 2009 at 05:09:10PM +0200, Johannes Hofmann wrote:
On Wed, Sep 30, 2009 at 04:31:42AM +0000, corvid wrote:
Johannes wrote:
On Sun, Sep 27, 2009 at 06:47:31PM +0000, corvid wrote:
Also, I tried moving the title attribute check to Html_parse_common_attrs because it's part of %coreattrs (as the html spec puts it), and styleengine's style0() gave me an assertion because now I'm doing too much style computation. What should I do in light of this?
Currently you can call StyleEngine::setNonCssHints() only once per element. So we would need to pass the CssPropertyList that holds the NonCssHints until we have collected them all in html.cc and then set them once.
Another thing to think about if we change the attr stuff around is that gprof shows me (unoptimized, auto img loading off, remote css off, embedded css on)
% cumulative self self total time seconds seconds calls ms/call ms/call name 7.63 2.26 2.26 259353 0.01 0.01 Html_get_attr2(DilloHtml*, char const*, int, char const*, int) 4.23 3.51 1.25 rgb_to_565d(unsigned char const*, unsigned char*, int, int) 2.40 4.22 0.71 329489 0.00 0.00 dw::Textblock::wordWrap(int) 2.27 4.89 0.67 194573 0.00 0.01 CssStyleSheet::apply(CssPropertyList*, Doctree*, DoctreeNode const*) 1.93 5.46 0.57 382 1.49 9.67 Html_write_raw(DilloHtml*, char*, int, int) 1.62 5.94 0.48 369132 0.00 0.00 lout::object::ConstString::hashValue(char const*) 1.32 6.33 0.39 149410 0.00 0.01 dw::core::Widget::queueResize(int, bool) 1.22 6.69 0.36 745628 0.00 0.00 Html_tag_compare(char const*, char const*) ..
I remember Jorge saying something once about having experimented with a more sophisticated/complicated get_attr system long ago but it not having been worth the complexity at the time.
Ah yes, I also thought about parsing attrs in one go and storing the result in some data structure. Whether this is really faster than the current approach needs to be tested.
I'm not opposed to check for a title attribute in every element, we just need to make sure it doesn't slow things down too much. Cheers, Johannes
participants (3)
-
corvid＠lavabit.com -
Johannes.Hofmann＠gmx.de -
mr_semantics＠hotmail.com