[Webkit-unassigned] [Bug 246852] [GTK] Crash on authentication dialog with GTK 4

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=246852

--- Comment #12 from enometh at meer.net ---
(In reply to Michael Catanzaro from comment #10)

>> Source/WebKit/UIProcess/API/gtk/WebKitAuthenticationDialog.cpp:194
>> +    if (tmplabel)
>> +        gtk_label_set_line_wrap(GTK_LABEL(tmplabel), TRUE);

> So the problem here is gtk_check_button_get_child() returns nullptr?
> Presumably it always returns nullptr, so the `if (tmplable)` is
> designed to catch a case that will never be hit, correct? Looks like
> this code here just needs to be removed or replaced with some other
> way of doing things.

Yes. I'm pretty sure it always returns null. I would replace the #if
USE(GTK) block with just that comment and no code, but perhaps that is
bad style to have a block with no code.

Let me know if you want to remove the gtk4 section and just have a if
!USE(GTK4) block.

> > Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp:1863
> > +// ;madhu 221105 focus is not a valid signal for a Dialog Window in gtk4. use move-focus instead
>
> Please remove all your personal comments, as they don't belong in the final version of the code.

Yes of course. Should I leave a comment here that says
// focus is not a valid signal for a Dialog Window in gtk4. use move-focus instead
or is it the commit message enough? I'd leave it in

> > Source/WebKit/UIProcess/API/gtk/WebKitWebViewDialog.cpp:128
> > +//;madhu 221105
> > +    gtk_widget_set_can_focus(GTK_WIDGET(object), TRUE);
>
> Style nit: add an extra blank line here.
>
> BTW you know this is a GTK 3 block, right? Why are you changing this
> for GTK 3?

I'll just remove this altogether. the commit message for this says
this is probably unnecessary, I probably thought this was required
when reading the gtk-doc for the function since i didn't know if the
default state was to "can_set_focus" TRUE, but if it worked before it
should be OK.

Thanks for the comments,I'll try to submit a new patch once I get your feedback on the 2
points

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20221115/3711ba6e/attachment.htm>