Re: [evolution-patches] GTKHTML Patch for #306238 (new)

[Kaushal Kumar <kakumar novell com>]

Hi Mengjie,

sorry about some delay in review. I was busy completing the remaining 
GAL retirement work.

> http://bugzilla.gnome.org/show_bug.cgi?id=306238
> The patch for this bug is also available on the above URL.
>
> As to your proposal, I signal the cursor position and get the offset of the text
> inserted or deleted in the a11y part. 
Okay.

> Because gnopernicus need to check the text deleted before it is really deleted,
> I emit the text_changed::delete signal before the deleing really happens.
Fine.

Please find some comments inline below:-

> @@ -362,7 +366,7 @@ gtk_html_a11y_insert_object_cb (GtkWidge
>  	}
>  
>  	if (G_IS_HTML_A11Y_TEXT(a11y)) {
> -		g_signal_emit_by_name (a11y, "text_changed::insert", pos, len);
> +		g_signal_emit_by_name (a11y, "text_changed::insert", cursor->offset - len, len);

Would the value of cursor->offset - len be passed as negative when we 
are at the beginning of the line and delete a character. (please refer 
my comments in the bug report for a similar situation in the last patch).

>  
>  	} 
>  }
> @@ -371,6 +375,7 @@ static void
>  gtk_html_a11y_delete_object_cb (GtkWidget * widget, int pos, int len) 
>  {
>  	AtkObject * a11y, *obj;
> +	HTMLCursor *cursor = GTK_HTML (widget)->engine->cursor;
>  
>          obj = gtk_widget_get_accessible (widget);
>  	a11y = gtk_html_a11y_get_focus_object (widget);
> @@ -383,7 +388,7 @@ gtk_html_a11y_delete_object_cb (GtkWidge
>  	}
>  
>  	if (G_IS_HTML_A11Y_TEXT(a11y)) {
> -		g_signal_emit_by_name (a11y, "text_changed::delete", pos, len);
> +		g_signal_emit_by_name (a11y, "text_changed::delete", cursor->offset, len);
do we need to minus len from cursor->offset being passed here?

>  	}
>  }
>  
> Index: text.c
> ===================================================================
> RCS file: /cvs/gnome/gtkhtml/a11y/text.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 text.c
> --- text.c	17 May 2005 14:47:56 -0000	1.15
> +++ text.c	21 Jun 2005 09:12:10 -0000
> @@ -532,29 +532,39 @@ html_a11y_text_get_text_at_offset (AtkTe
>  	switch (boundary_type) {
>  	case ATK_TEXT_BOUNDARY_LINE_START:
>  		start_slave = html_text_get_slave_at_offset (to, NULL, offset);
> -		g_return_val_if_fail (start_slave, NULL);
> -		end_slave = (HTMLTextSlave *) HTML_OBJECT (start_slave)->next;
>  
> -		if (end_slave && HTML_IS_TEXT_SLAVE (end_slave)) {
> -			*end_offset = end_slave->posStart;
> +		if (start_slave == NULL) {
> +			*start_offset = 0;
> +			*end_offset = to->text_len;
could you elaborate on these offset values set as above. Just needed to 
understand it better. Earlier we used to return on a null start_slave. 
Now, we proceed with these values.
- can we proceed with a null start_slave instead of returning null?
- how do we decide on these preset values of offsets?

>  	case ATK_TEXT_BOUNDARY_LINE_END:
>  		end_slave = html_text_get_slave_at_offset (to, NULL, offset);
> -		g_return_val_if_fail (end_slave, NULL);
> -		start_slave = (HTMLTextSlave *) HTML_OBJECT (end_slave)->prev;
>  
> -		if (start_slave && HTML_IS_TEXT_SLAVE (start_slave)) {
> -			*start_offset = start_slave->posStart + start_slave->posLen;
> +		if (end_slave == NULL) {
> +			*start_offset = 0;
> +			*end_offset = to->text_len;

same query as above for this change.

> --- htmlengine-edit-cut-and-paste.c	13 Apr 2005 15:35:32 -0000	1.111
> +++ htmlengine-edit-cut-and-paste.c	21 Jun 2005 08:57:38 -0000
> @@ -1769,6 +1769,10 @@ html_engine_delete (HTMLEngine *e)
>  		gint start_position = start->position;
>  		gint end_position = end->position;
>  
> +		if (end_position - start_position > 0) {
> +			int len = end_position - start_position;
> +			g_signal_emit_by_name (e->widget, "object_deleted", start_position, len);
> +		}
>  
So we emit the object_deleted signal before the delete actually happens.
Radek: Would you kindly give your opinion. Thanks.

>  		while (start->position < end->position) {
>  			if (check_for_simple_delete (start->object, end->object)) {

--
Kaushal Kumar  <kakumar novell com>