After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 750842 - Fixes to the form filler
Fixes to the form filler
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-06-12 12:13 UTC by Claudio Saavedra
Modified: 2015-06-16 10:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
web-extension: use proper number format for position dom properties (4.62 KB, patch)
2015-06-12 12:13 UTC, Claudio Saavedra
committed Details | Review
web-extension: fix color of form filler popup (2.11 KB, patch)
2015-06-12 12:13 UTC, Claudio Saavedra
committed Details | Review

Description Claudio Saavedra 2015-06-12 12:13:45 UTC
It currently doesn't display properly because of wrong numeric types and not using GtkStyleContext properly. Attached patches fix both issues.
Comment 1 Claudio Saavedra 2015-06-12 12:13:51 UTC
Created attachment 305139 [details] [review]
web-extension: use proper number format for position dom properties

DOM bindings use double, not long. This fixes the form filler
popup not showing up.
Comment 2 Claudio Saavedra 2015-06-12 12:13:56 UTC
Created attachment 305140 [details] [review]
web-extension: fix color of form filler popup

Style context calls were missing setting the state previously
as required by GtkStyleContext.
Comment 3 Carlos Garcia Campos 2015-06-12 13:26:53 UTC
Review of attachment 305139 [details] [review]:

::: embed/web-extension/ephy-web-extension.c
@@ +621,3 @@
   style_attribute = g_strdup_printf ("position: absolute; z-index: 2147483647;"
                                      "cursor: default;"
+                                     "width: %lfpx;"

%lf exist?

::: lib/ephy-web-dom-utils.c
@@ +452,2 @@
   g_object_get (element,
                 "offset-left", &offset_left,

This is one of the reasons why I don't like g_object_get(), I would also change this to use the public getters instead (webkit_dom_element_get_offset_left/top).
Comment 4 Carlos Garcia Campos 2015-06-12 13:27:20 UTC
Review of attachment 305139 [details] [review]:

::: embed/web-extension/ephy-web-extension.c
@@ +621,3 @@
   style_attribute = g_strdup_printf ("position: absolute; z-index: 2147483647;"
                                      "cursor: default;"
+                                     "width: %lfpx;"

%lf exist?

::: lib/ephy-web-dom-utils.c
@@ +452,2 @@
   g_object_get (element,
                 "offset-left", &offset_left,

This is one of the reasons why I don't like g_object_get(), I would also change this to use the public getters instead (webkit_dom_element_get_offset_left/top).
Comment 5 Carlos Garcia Campos 2015-06-12 13:28:50 UTC
Review of attachment 305140 [details] [review]:

Yes, this changed in GTK+ :-/ don't we need to save/restore the context?
Comment 6 Claudio Saavedra 2015-06-12 15:52:17 UTC
Review of attachment 305139 [details] [review]:

::: embed/web-extension/ephy-web-extension.c
@@ +621,3 @@
   style_attribute = g_strdup_printf ("position: absolute; z-index: 2147483647;"
                                      "cursor: default;"
+                                     "width: %lfpx;"

%lf is for double, no?

::: lib/ephy-web-dom-utils.c
@@ +452,2 @@
   g_object_get (element,
                 "offset-left", &offset_left,

Good point.
Comment 7 Claudio Saavedra 2015-06-12 15:53:08 UTC
Review of attachment 305140 [details] [review]:

Because of how we use this context, we don't need to, but it would be theoretically better.
Comment 8 Claudio Saavedra 2015-06-16 10:45:29 UTC
Attachment 305139 [details] pushed as 8a3c7df - web-extension: use proper number format for position dom properties
Attachment 305140 [details] pushed as 772f289 - web-extension: fix color of form filler popup