From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#69941: 30.0.50; Faulty fontification of radio button widgets Date: Sat, 06 Apr 2024 13:18:31 +0300 Message-ID: <86plv23ibs.fsf@gnu.org> References: <87h6gynx49.fsf@rub.de> <865xxe1dwd.fsf@gnu.org> <51c20b56-4b82-4f5c-9559-cdbd0146df22@gmail.com> <87wmprqxj3.fsf@gmx.net> <8734s5w1mf.fsf@gmx.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="406"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 69941@debbugs.gnu.org, monnier@iro.umontreal.ca To: maurooaranda@gmail.com, Stephen Berman Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Apr 06 12:19:14 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rt38k-000AQE-2o for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 06 Apr 2024 12:19:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rt38c-000128-Pt; Sat, 06 Apr 2024 06:19:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rt38a-00011w-K8 for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 06:19:04 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rt38S-0007G8-Ba for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 06:19:04 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rt38Y-0002UY-Cd for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 06:19:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 Apr 2024 10:19:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69941 X-GNU-PR-Package: emacs Original-Received: via spool by 69941-submit@debbugs.gnu.org id=B69941.17123987289488 (code B ref 69941); Sat, 06 Apr 2024 10:19:02 +0000 Original-Received: (at 69941) by debbugs.gnu.org; 6 Apr 2024 10:18:48 +0000 Original-Received: from localhost ([127.0.0.1]:38357 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt38J-0002Sy-OR for submit@debbugs.gnu.org; Sat, 06 Apr 2024 06:18:48 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53318) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt38I-0002S5-0M for 69941@debbugs.gnu.org; Sat, 06 Apr 2024 06:18:46 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rt386-0007Dq-E6; Sat, 06 Apr 2024 06:18:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From: Date; bh=zmczGXcrhjuDj6JX8sr4zmTlpHuAHUt6GDGUom0SQsk=; b=J+VINpXNPH3ryUnuZTA7 44gbJXm9yXLNYD/gqdg+QveXys8USendaLClsxjlJsXnM5z2uDZ01m3t1hMoKwWg/UcK5H8P0IN3N jzjcb+FOLQ75ovHjiyhxiVKFrFxY66w7p+JpSu04X0PZm3Lyhx/1XuJyvXUm/FOnHyEzgxlx7BErg yn44Thtqhvoi9H3WiB31cVpeaZuVuVi63dKCUAS+njMuJGwgozg3txuW4FPmaqakGTxdXDIQ7i+Au Ff03qt/Kd0z770p1Pjj9ubmc2LcRBR8eRhz/vK107f+WyiU58Y4gTQ8Tm2C1C8hN/HIM+Cn8RweIe zNbFLfhE/LJXYg==; In-Reply-To: <8734s5w1mf.fsf@gmx.net> (message from Stephen Berman on Mon, 01 Apr 2024 17:20:40 +0200) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:282776 Archived-At: Ping! > From: Stephen Berman > Cc: Eli Zaretskii , 69941@debbugs.gnu.org, Stefan Monnier > > Date: Mon, 01 Apr 2024 17:20:40 +0200 > > On Sun, 24 Mar 2024 19:45:20 +0100 Stephen Berman wrote: > > > On Sat, 23 Mar 2024 17:49:53 -0300 Mauro Aranda wrote: > > > >> Stephen Berman writes: > >> > >>> 5. Tab back to "Activate" and press RET, again restoring the initial > >>> state.  Now tab to radio button "Two" and press RET. > >>> => The fontification is the same as in step 4: radio button "Two" has > >>> the widget-inactive face but radio button "One" has the default (active) > >>> face, though it is again inactive.  Repeatedly pressing either of the > >>> radio buttons (after activating them), does not change the fontification > >>> of "One" again. > >>> > >>> > >>> The faulty fontification of radio button "One" also obtains if there is > >>> just one radio button instead of two, and if there are more than two > >>> radio buttons, it is only the first one that displays the odd > >>> fontification (admittedly, I've only test up to three radio buttons). > >>> > >>> I've tried to debug this and found that the problem seems to be due to > >>> the sexp (set-marker-insertion-type from t) near the end of > >>> widget-default-create, which advances the marker specified by the > >>> widget's :from property.  Changing t to nil fixes the faulty > >>> fontification of the first radio button. > >>> > >>> I investigated the history of this code, and while the value t for the > >>> marker insertion type was used in the initial commit, it was changed to > >>> nil in commit e0f956935, with the message "Insert new text at the :from > >>> marker _after_ the marker, not before it."  But 18 days later it was > >>> changed back to t in commit 3bff434b8, that also added "Document need to > >>> put some text before the %v escape in :format string" of editable-field > >>> widgets.  (I looked at the bug-gnu-emacs and emacs-devel mailing list > >>> archives but found nothing relevant at the time just prior to these > >>> commits.) > >> > >> I'm pretty sure it makes sense for user-editable widgets that the > >> value for insertion-type be t. > > > > Yes, if my understanding is correct, it's just radio-button-choice > > widgets that need (the effect of) insertion type nil (at least for > > setting the widget-inactive face), see below. > > > >>> So evidently the advancing marker insertion type is needed for at least > >>> some widgets, though it seems to be problematic for radio buttons.  So I > >>> tried to conditionalize the choice of t or nil on the type of the > >>> widget.  I used (not (eq 'radio-button (widget-type widget))), since the > >>> argument `widget' of widget-default-create is, according to Edebug, > >>> indeed radio-button, so negating the eq sexp returns nil, which I had > >>> found to be the value of the marker insertion type that fixes the > >>> fontification (however, I couldn't think of a way of limiting the > >>> conditioning to only the first radio button, but in my testing so far > >>> that lack doesn't appear to make a difference). > >> > >> I'm not sure if the right target is the radio-button widget.  It could > >> be the radio-button-choice widget.  Did you try to conditionalize the code > >> against the radio-button-choice widget? > > > > I didn't, because I got hung up on the radio-button widget, since in > > Edebug that is what I saw and (mistakenly) took to be the current widget > > when widget-inactive face is set. But the resulting marker insertion > > type discrepancy is really proof that I was looking at the wrong widget > > type (as I already realized in my comments cited below, but I didn't > > think to simply try it with radio-button-choice until now, so thanks for > > pointing me in the right direction!). And indeed, with > > radio-button-choice, negating the eq test DTRT, i.e., using (not (eq > > 'radio-button-choice (widget-type widget))) as the condtion results in > > the correct fontification. Since this sexp gives the > > radio-button-choice widget's :from property the marker insertion type > > nil, there is no discrepancy between using that sexp and directly using > > nil, so changing my patch to use that condition would be in improvement. > > Alternatively, ... > > > >>> But in fact, using the negation of the value of the eq sexp results in > >>> the same faulty fontification, while omitting the negation (as in the > >>> attached patch), which yields the advancing insertion type t, gives the > >>> correct fontification, just like using nil does.  This makes no sense to > >>> me, yet it is reliably reproducible.  The only possible explanation that > >>> occurs to me is that the bug is triggered elsewhere in the Emacs code > >>> and somehow using the sexp that evaluates to t as the marker insertion > >>> type affects that code, while using t itself does not (or rather, has > >>> the opposite effect); but how that could be and where the culpable code > >>> is, I don't know (as a guess, perhaps in the C code that adds faces, but > >>> I don't know how to debug that).  If anyone knows or has an idea what's > >>> going on here, please communicate it.  In the meantime I will continue > >>> to use the widget library with the patch to see whether it has unwanted > >>> consequences. > >> > >> I don't know much about that code in Emacs.  If we find some hack that > >> works maybe we can use that until someone figures it out.  But again, > >> given your analysis, I'd like to find out if using the condition on the > >> radio-button-choice widget works as expected.  And of course, the hack > >> shouldn't be added to the widget-default-create, which should remain > >> type agnostic. > > > > ... since the issue is fontification with the widget-inactive face, > > perhaps a better location for the condition is widget-specify-inactive, > > as in the attached patch. It's still a hack though, since > > widget-specify-inactive is also type-agnostic by design. But if the > > issue really is confined to radio-button-choice widget's, I guess any > > solution will have to refer to that type. However, between adding the > > condition to widget-specify-inactive or to widget-default-create, I'm > > not sure which is less hacky: since the patch to widget-default-create > > effectively undoes the result of setting the marker insertion type to t, > > perhaps it is cleaner just to set it to nil for radio-button-choice > > widgets in widget-default-create. Or maybe someone will come up with a > > better fix... > > > > Steve Berman > > > > diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el > > index 172da3db1e0..01319853edc 100644 > > --- a/lisp/wid-edit.el > > +++ b/lisp/wid-edit.el > > @@ -532,6 +532,17 @@ widget-inactive > > > > (defun widget-specify-inactive (widget from to) > > "Make WIDGET inactive for user modifications." > > + ;; When WIDGET is a radio-button-choice widget and its first child > > + ;; radio-button widget is inserted, the marker FROM, which has > > + ;; insertion type t, advances to the position after the radio button, > > + ;; and since the overlay setting the widget-inactive face begins at > > + ;; the position of FROM, this results in the first radio button > > + ;; incorrectly not being fontified with the widget-inactive face. To > > + ;; ensure it is correctly fontified, we move FROM backward by 3, > > + ;; i.e. the length of the radio-button widget (from its string > > + ;; representation "( )" or "(x)") (bug#69941). > > + (when (eq (widget-type widget) 'radio-button-choice) > > + (set-marker from (- from 3))) > > (unless (widget-get widget :inactive) > > (let ((overlay (make-overlay from to nil t nil))) > > (overlay-put overlay 'face 'widget-inactive) > > To fix this bug, do you have a preference between this patch for > widget-specify-inactive and the attached patch for > widget-default-create? Or do you have a better fix? > > Steve Berman