GNU bug report logs -
#69941
30.0.50; Faulty fontification of radio button widgets
Previous Next
Reported by: Stephen Berman <stephen.berman <at> gmx.net>
Date: Fri, 22 Mar 2024 15:01:01 UTC
Severity: normal
Found in version 30.0.50
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Full log
Message #101 received at 69941 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> I attach a patch that implements something like that, along with a new
>> test to check the recursive case.
>
> Great. A few more comments:
>
>> + (let* ((parent (widget-get widget :parent))
>> + (parent-from-marker (and parent (widget-get parent :from)))
>> + (parent-to-marker (and parent (widget-get parent :to)))
>> + (continue nil)
>> + (lst nil)
>> + (pos (point)))
>> + (when (and parent-from-marker
>> + (eq pos (marker-position parent-from-marker)))
>> + (set-marker-insertion-type parent-from-marker nil)
>> + (push (cons parent-from-marker t) lst)
>> + (setq continue t))
>> + (when (and parent-to-marker
>> + (eq pos (marker-position parent-to-marker)))
>> + (set-marker-insertion-type parent-to-marker t)
>> + (push (cons parent-to-marker nil) lst)
>> + (setq continue t))
>> + (when continue
>> + (setq lst
>> + (append lst (widget--prepare-markers-for-inside-insertion
>> parent))))
>> + lst))
>
> It just occurred to me that if widget updates happen recursively
> (meaning that another widget update happens between the two calls to
> `widget--prepare-markers-for-*-insertion`), then the
> `widget--prepare-markers-for-outside-insertion` done at the end
> of the inner call will mean that any insertion that happens afterwards
> can end up inserting "outside" instead of "inside". So, I think to
> protect against that, we need the above code to push only those markers
> which we do change (i.e. we need to test the current value of
> `marker-insertion-type` before pushing it to `lst`).
> Does that make sense, or am I imagining things?
>
I think I get what you mean. It makes sense, although I couldn't figure
out a precise scenario where this might actually happen. I'll try to
explain:
Recursive updates happen anytime we are creating a composite
widget, and I think the situation you describe could potentially happen
when we are recreating a composite widget that's inside another
composite widget and that should be common enough to worry about it.
So we have: composite-widget > composite-widget2 > children.
If we're recreating the 'composite-widget2', then it will have markers
pointing nowhere (because of the deletion that happens first in
`widget-default-value-set`). So the code will prepare the markers of
'composite-widget', but when `widget-default-create` gets called for
creating 'children', `widget--prepare-markers-for-inside-insertion` will
return an empty list, effectively avoiding the scenario you though
about, IIUC.
Nevertheless, since my imagination can only go so far, and because it's
quite easy to add those checks, I implemented that in the attached
patch.
> PS: While I'm here, I might as well mention the irrelevant tweaks
> I'd be tempted to do:
>
> - Maybe instead of "prepare" I'd use "revert" in the name of the
> second function.
> - `continue` is non-nil exactly when `lst` is non-nil, so we can drop
> `continue`.
> - Since `lst` is a freshly consructed list, we can use `nconc`.
> - The `setq` at the end is right before returning `lst`
> so we don't need it, instead we can finish with:
>
> (when lst
> (nconc lst (widget--prepare-markers-for-inside-insertion
parent)))))
These are most welcome, thanks. New patch attached.
[0001-Prepare-markers-for-insertions-inside-of-a-widget.patch (text/x-patch, attachment)]
This bug report was last modified 82 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.