From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Mauro Aranda Newsgroups: gmane.emacs.bugs Subject: bug#69941: 30.0.50; Faulty fontification of radio button widgets Date: Thu, 18 Apr 2024 07:18:29 -0300 Message-ID: <941f6565-203b-47bf-82a9-2bb7b0788b6a@gmail.com> 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; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35858"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla Thunderbird Cc: Eli Zaretskii , 69941@debbugs.gnu.org, Stefan Monnier To: Stephen Berman Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Apr 18 12:20:18 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 1rxOsL-0009Bt-9S for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 18 Apr 2024 12:20:17 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rxOs5-0003V8-NN; Thu, 18 Apr 2024 06:20:01 -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 1rxOs3-0003V0-Du for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 06:19:59 -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 1rxOs3-0006G7-69 for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 06:19:59 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rxOsG-0007lI-4G for bug-gnu-emacs@gnu.org; Thu, 18 Apr 2024 06:20:12 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Mauro Aranda Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 18 Apr 2024 10:20:11 +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.171343556529527 (code B ref 69941); Thu, 18 Apr 2024 10:20:11 +0000 Original-Received: (at 69941) by debbugs.gnu.org; 18 Apr 2024 10:19:25 +0000 Original-Received: from localhost ([127.0.0.1]:51532 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rxOrI-0007fb-Ec for submit@debbugs.gnu.org; Thu, 18 Apr 2024 06:19:23 -0400 Original-Received: from mail-pf1-x434.google.com ([2607:f8b0:4864:20::434]:54766) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rxOqy-0007cb-LB for 69941@debbugs.gnu.org; Thu, 18 Apr 2024 06:19:07 -0400 Original-Received: by mail-pf1-x434.google.com with SMTP id d2e1a72fcca58-6ed627829e6so823923b3a.1 for <69941@debbugs.gnu.org>; Thu, 18 Apr 2024 03:18:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1713435513; x=1714040313; darn=debbugs.gnu.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=di0O0Y/jxf1MxlJfQpo5y/d3Uwp9OyL0E9hr5ywyOAQ=; b=PnR/PGoes0jkYaxCY2FFZKismneeAdFOuLhtcZJ4Zg4o7embkvYNV9VnJ/aeVStV8s NBHUkXhiEs7qgE6xO7lr3KnpJjcQVXkA60Sg8hXYJABifL8KYSoHnjfFNAcbQZ4XivsP 8FMzFBhIzithz4vFxmxscJUU5xyoxlGonf4Y58B1nate3os5jXY++vqIzuZSOIrOIy4i m2JH5Bge2sdbA8ahZ0n+RYX9HPkNEgjsDaQtCJVs5u7G2k5QTegmD4QAaHSfGYcauBj5 E/EiMhCpgjOKOfwboDKiAVvXfAxA6SZMtX/ys6pfonKFgKb3MATc2m2b5/g2YG777Z5/ 7fHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713435513; x=1714040313; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=di0O0Y/jxf1MxlJfQpo5y/d3Uwp9OyL0E9hr5ywyOAQ=; b=qBOo0ewD8ZwUnW0fjYcsh9QWBlH2X2dinjOEQ8vWQ73e/FZYLw0b3QvmU5Q1Qn7wrc Nj8g8UQtQIUrMyh4rdUa9WjBfBEX0nRwZUrfaMisfRzPp7EJ7JK+VMvpVwiwqmDoLauv hvxA/AgMM7PSeKVpvV6UxGFYJLITmcedFOWmzv3KAax4BTAsmtXi7VMDVA3hbpqZiZ4L FotbAay/QDBJSV2M9omb2p6h8puq54LwC/buKZmDwt+3oI9nU+fysBhd3zVmxS1/j9Ce o3UgEZYeMML5uB51sCEcVVo0lskKSrLj+LnVHdn6yRx5xVgi1vehie4E8pq+6kCt6LDv KWzQ== X-Forwarded-Encrypted: i=1; AJvYcCVSIF0Sy3x99KFQJVU3lkAIIeEO92ou9m4V3q/FYiDIFM5/mKJ6YUi8uEWR/w8dpIdqND9W+MnKbobXMp911/ex67/cpbM= X-Gm-Message-State: AOJu0YxUHmg8ndShAUvqtRGT3NiQLLZXvsrCn+vV+uvk5imWJOjH5kGW +VhuIuIShqefw8qBgtmF5gQMn3H5MBFhehhYF9bSYDUGMQsBtRAVQQ/8Vw== X-Google-Smtp-Source: AGHT+IHoyw+Sd/eDQuXnSum+fheMYFXNTFRq3jDYqe43SJesRlT5hJnRSH7chhFHQ3XqzYUC4D/vyQ== X-Received: by 2002:a05:6a21:8015:b0:1a9:a32c:f6d6 with SMTP id ou21-20020a056a21801500b001a9a32cf6d6mr2327151pzb.55.1713435512900; Thu, 18 Apr 2024 03:18:32 -0700 (PDT) Original-Received: from [192.168.0.100] ([181.228.33.6]) by smtp.gmail.com with ESMTPSA id u3-20020a631403000000b005bdbe9a597fsm1098165pgl.57.2024.04.18.03.18.31 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 18 Apr 2024 03:18:32 -0700 (PDT) Content-Language: en-US In-Reply-To: <8734s5w1mf.fsf@gmx.net> 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:283543 Archived-At: > 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 I don't really have a better fix.  I mean, ideally, we'd find the reason why the setting behaves differently for the radio-button-choice widget, and only for the first one in a radio widget, as it seems to me. But I'll need more time to be able to look into that. That said, if Eli is OK with installing a minor hack (with a FIXME, please), I don't have problems.  And since it's a hack (and hopefully temporary), it's better if we keep it at widget-default-create then.