[patch]: trim the labels of the stylesheet menu
While thinking about stylesheet issues I noticed a UI problem: the stylesheet menu uses URLs as the labels of the menu items. This is unfortunate because some sites have stylesheets with insanely long URLs. (Try Wikipedia if you don't believe me!) So I'm attaching a patch that trims long URLs in the menu. This is just a UI change - no functionality is affected. Here's a demonstration of the patch on the front page of Wikipedia: http://starurchin.org/dillo/stylesheet_menu/ Notes: Since the label is no longer always the same as the label the callback must find the URL elsewhere. We put it in the user_data() of the menu item. We just copy the pointer to the DilloUrl structure, so we are assuming it will never be free()-ed while the menu is up. If I'm wrong about this the patch will need to be revised. We get a nice cleanup for free: we loose the mysterious magic constant 5 in the callback. (Bonus points if you can see what that magic constant is! Double bonus points if you can see why the code still worked when I erroneously left it in!) Please comment, Jeremy Henty
Jeremy wrote:
While thinking about stylesheet issues I noticed a UI problem: the stylesheet menu uses URLs as the labels of the menu items. This is unfortunate because some sites have stylesheets with insanely long URLs. (Try Wikipedia if you don't believe me!) So I'm attaching a patch that trims long URLs in the menu. This is just a UI change - no functionality is affected.
Here's a demonstration of the patch on the front page of Wikipedia:
Good. It would be nice to improve the labels. It's interesting that FLTK decides to clip a little bit from the end there. I wonder what it was thinking...
Notes:
Since the label is no longer always the same as the label the callback must find the URL elsewhere. We put it in the user_data() of the menu item.
We just copy the pointer to the DilloUrl structure, so we are assuming it will never be free()-ed while the menu is up. If I'm wrong about this the patch will need to be revised.
I don't know the details of when Jorge's code decides to free cache entries, but I think it would be best not to assume that the url remains valid.
On Fri, Nov 20, 2009 at 09:28:25PM +0000, corvid wrote:
Jeremy wrote:
It's interesting that FLTK decides to clip a little bit from the end there. I wonder what it was thinking...
Me too, but I didn't feel like wading through FLTK code tonight! :-)
We just copy the pointer to the DilloUrl structure, so we are assuming it will never be free()-ed while the menu is up. If I'm wrong about this the patch will need to be revised.
I don't know the details of when Jorge's code decides to free cache entries, but I think it would be best not to assume that the url remains valid.
OK, I agree: robust code should ensure its own safety, not depend on other code to look after it. Here's a patch to do that. It applies on top of the previous patch. Jeremy
Jeremy wrote:
On Fri, Nov 20, 2009 at 09:28:25PM +0000, corvid wrote:
Jeremy wrote:
It's interesting that FLTK decides to clip a little bit from the end there. I wonder what it was thinking...
Me too, but I didn't feel like wading through FLTK code tonight! :-)
Turns out that it doesn't do it to me when I run it. So let's blame the font...
We just copy the pointer to the DilloUrl structure, so we are assuming it will never be free()-ed while the menu is up. If I'm wrong about this the patch will need to be revised.
I don't know the details of when Jorge's code decides to free cache entries, but I think it would be best not to assume that the url remains valid.
OK, I agree: robust code should ensure its own safety, not depend on other code to look after it. Here's a patch to do that. It applies on top of the previous patch.
Committed.
participants (2)
-
corvid＠lavabit.com -
onepoint＠starurchin.org