I'm trying to fully understand this to see what we can add in the WebKit API. If I understand correctly, the mail preview will no longer be a single web view, but a vertical box with widgets and web views. We want everything scrollable under the same GtkScrolledWindow. Since web views are scrollable by themselves they always claim a minimum size of 0x0, but we need to make them not scrollable and always claim a minimum size of 0xcontents-height. Right? What I don't get is why the contents of the web view can changed, they represent a message body or attachment, which are both inmutable, no? So, would it be enough to expose the contents size so that you can force a size request on the view? We could make it a web view property that you can monitor with notify signal. Or would it be better to add a mode to web view to do that automatically? It could make get_preferred_height to always return the contents height, and probably set the page in fixed layout mode. Tomas, maybe you can explain the use case in the webkitgt-gtk mailing list so that we can discuss how to provide a good API for it in there.

(In reply to Carlos Garcia Campos from comment #1)
> ... Right?

Correct. The message preview would be like this:

  GtkScrolledWindow
    - GtkBox::vertical
      - headers (WebView)
      - EAttachmentBar (GtkWidget)
      - message body (WebView)
      - attachment 1 [details] (GtkWidget)
        - preview (GtkWidget or WebView) (*)
      - attachment 2 [details] (GtkWidget)
        - preview (GtkWidget or WebView) (*)
      ...

(*) the 'preview' can be for an attached message, thus it is, again,
    a structure starting at GtkBox::vertical

> What I don't get is why the contents of the web view can changed,
> they represent a message body or attachment, which are both inmutable, no?

No. Evolution changes content of the web view on the fly:
a) message headers - there is a [+] button, which can hide large headers and
   show only "<b>subject</b> (from)" instead (aka collapse headers), where
   the rich headers and those collapsed are managed through 'display' CSS
   property, thus the [+] button click doesn't need page reload
b) sender images - these are resolved asynchronously, and can take a long
   time eventually
c) Meeting invitations - these are even better, they add new text in
   the middle of the "page", both when it failed to open some calendar (then
   a warning is shown), or when user decides to save his changes (accept/
   decline/tentative) for the meeting, then an information whether the save
   succeeded is added into the page, eventually together with the previous
   warnings being removed
d) vCard attachments - there are two buttons to show a simple contact
   or a full contact information; similar with the headers
e) prefer plain - allows to switch between the text/html and text/plain
   part on the fly (through context menu)
f) text hightlight - syntax highlight changes an iframe src attribute which
   generates different view on a text/plain part; it's similar as prefer plain

These are the few I can think of right now.

> Or would it be better to add a mode to web view to do that automatically?

I think this will be better, because you avoid theoretical code duplication in applications where they'd prefer to have WebView non-scrollable.

This auto-hight reminded me of bug #739955, where we face crashes when trying to change properties on the fly based on the view width. It can be fixed with webkit2, I do not know, it's that only the issue we try to solve here is too close to the other one (at least from my point of view). I mean with that, I would prefer if the widget's hight request would be done on the side of the WebKit, thus it'll not cause crashes. Writing a notify::content-height property change listener is easy, but it's similar to what we tried in this other bug report and it had issues. If you think that I mix unrelated things together, then I'm sorry and do not be confused, please.

Created attachment 326484 [details]
test-wk2.c

Simple example, which packs a GtkTextView on one side and two WebKitWebView-s on the other side of the GtkPaned, while the web views are packed in a scrollable window and a vertical box. The first line contains a commend with a command line to compile and run it. This requires KaL's test patch I received from him for the webkitgtk.

My initial test shows two issues with WEBKIT_SIZE_REQUEST_CONTENTS_HEIGHT (the one I'm interested in):

a) run it and change the width of the right side (by dragging the middle handle of the GtkPaned) to as small as the longest word doesn't fit the width. Once the limit is reached, there are shown both scrollbars in the bottom web view. There shouldn't be shown any, ideally, the offending width should be used as the minimal width and the scrolling should be left for the parent GtkScrollableWindow. The vertical scrollbar also seems to be there only to cover the space held by the horizontal scrollbar. In any case, no web view's scrollbar is expected here.

b) when dragging the paned handle back to the left, thus giving more space to the right side, the text is re-wrapped properly, but the height is left as the maximum it was. It changes, by line, only when making the right side smaller again (by dragging the handle). It's a nice semi-animation effect, as it resized its height by line, but otherwise nothing what one would want :)

I tried also with WEBKIT_SIZE_REQUEST_CONTENTS, and I see that the width of the web view never goes smaller than it was, thus enlarging it with the handle and then making it smaller will turn on horizontal scrollbar of the parent GtkScrolledWindow, while it should rather allow text wrapping and changing the height. Similar behaviour with the CONTENTS_WIDTH.

(In reply to Milan Crha from comment #3)
> Created attachment 326484 [details]
> test-wk2.c
> 
> Simple example, which packs a GtkTextView on one side and two
> WebKitWebView-s on the other side of the GtkPaned, while the web views are
> packed in a scrollable window and a vertical box. The first line contains a
> commend with a command line to compile and run it. This requires KaL's test
> patch I received from him for the webkitgtk.
> 
> My initial test shows two issues with WEBKIT_SIZE_REQUEST_CONTENTS_HEIGHT
> (the one I'm interested in):
> 
> a) run it and change the width of the right side (by dragging the middle
> handle of the GtkPaned) to as small as the longest word doesn't fit the
> width. Once the limit is reached, there are shown both scrollbars in the
> bottom web view. There shouldn't be shown any, ideally, the offending width
> should be used as the minimal width and the scrolling should be left for the
> parent GtkScrollableWindow. The vertical scrollbar also seems to be there
> only to cover the space held by the horizontal scrollbar. In any case, no
> web view's scrollbar is expected here.

This is the expected behavior of WEBKIT_SIZE_REQUEST_CONTENTS_HEIGHT, the vertical scrolling is delegated to the GtkScrolledwindow, but every web view has its own horizontal scrolling, since they could have different sizes. If that's not the behavior desired for evo, the we should use WEBKIT_SIZE_REQUEST_CONTENTS instead.
 
> b) when dragging the paned handle back to the left, thus giving more space
> to the right side, the text is re-wrapped properly, but the height is left
> as the maximum it was. It changes, by line, only when making the right side
> smaller again (by dragging the handle). It's a nice semi-animation effect,
> as it resized its height by line, but otherwise nothing what one would want
> :)

In my case it never shrinks again. This looks like a WebCore issue, the contents size we are receiving has the maximum height, we need to take a look at how contents size are calculated in WebCore.

> I tried also with WEBKIT_SIZE_REQUEST_CONTENTS, and I see that the width of
> the web view never goes smaller than it was, thus enlarging it with the
> handle and then making it smaller will turn on horizontal scrollbar of the
> parent GtkScrolledWindow, while it should rather allow text wrapping and
> changing the height. Similar behaviour with the CONTENTS_WIDTH.

Yes, this is because I didn't take into account that in the case of width, the size is actually dynamic because of the text wrapping. The current patch claims the current contents size as the minimum size. When resizing the pane, GTK+ doesn't call size_allocate on the wen view because we are not supposed to be able to become smaller. In the case of width I think we could report 0 as the minimum size while the contents size fit in the allocated size, and the contents size when it doesn't. Let me prepare a new patch.

I'm trying a new approach, but I'm a bit lost. The idea is to claim 0 as minimum size while the contents fit in the allocated size, so that the widget is resizable. The problem is that as soon as the contents don't fit in the allocated size, gtk+ doesn't ask the preferred width anymore, and the last width it has cached is 0. We don't know the minimum layout size in advance, so we need to allow gtk+ to resize the widget, and then check the contents size for the new allocated size.

Created attachment 326868 [details]
updated incomplete wk.patch

I updated your webkit patch slightly, by adding the lines to set zero minimum_width to the webkitWebViewBaseGetPreferredWidth/Height if the content size can be viewed in the allocated width/height. I also added some debug prints there. This patch is applicable to git master of webkit at commit_4a5c886ddfaf, which claims "git-svn-id: http://svn.webkit.org/repository/webkit/trunk@200122 268f45cc-cd09-0410-ab3c-d52691b4dbfc" here.

From what I see with the test-wk2.c and the WEBKIT_SIZE_REQUEST_CONTENTS (I changed to that, as it seems to be the one which will be used in the evolution), "adding" to the previously mentioned issues:
c) it still happens that the scrollbars in the respective web views are
   shown - that should be happening
d) when I make the width for the two web views small enough that the "looo..ng"
   word doesn't fit into the allocated width, then change the window height
   as much as only 3 lines of the text would fit the allocated part, then
   a busy-loop happens where the view will recalculate whether to show
   the internal scrollabars or not. The debug prints loop in the below states
   with the UI flashing:
> webkitWebViewBaseSetContentsSize: [198,73] ~> [211,73]
>    webkitWebViewBaseGetPreferredHeight: contentsSize:[211,73] out:nat:73 min:0 allocated:73
>    webkitWebViewBaseGetPreferredWidth: contentsSize:[211,73] out:nat:211 min:0 allocated:211
> webkitWebViewBaseSetContentsSize: [211,73] ~> [198,73]
>    webkitWebViewBaseGetPreferredHeight: contentsSize:[198,73] out:nat:73 min:73 allocated:63
>    webkitWebViewBaseGetPreferredWidth: contentsSize:[198,73] out:nat:198 min:0 allocated:211

   As the busy loop goes through the main loop, it can be avoided when
   the window size is changed again. I'm pretty sure that this is caused by
   the extra scrollbars inside the top web view, thus when the a)/c) is
   addressed the d) might be as well. 

I cannot reproduce the never-shrinking issue with this patch and the WEBKIT_SIZE_REQUEST_CONTENTS being used.

Regarding the get_preferred_width/height being called or not, it seems to me that it was the case only when the view couldn't change its size, thus in my case only if the given width for the two web views was not enough to be able to show the "looo...ng" word. That is, the width of the "looo...ng" word defines the width for both web views, which is correct, from my point of view.

(In reply to Milan Crha from comment #6)
> Created attachment 326868 [details]
> updated incomplete wk.patch
> 
> I updated your webkit patch slightly, by adding the lines to set zero
> minimum_width to the webkitWebViewBaseGetPreferredWidth/Height if the
> content size can be viewed in the allocated width/height. I also added some
> debug prints there. This patch is applicable to git master of webkit at
> commit_4a5c886ddfaf, which claims "git-svn-id:
> http://svn.webkit.org/repository/webkit/trunk@200122
> 268f45cc-cd09-0410-ab3c-d52691b4dbfc" here.
> 
> From what I see with the test-wk2.c and the WEBKIT_SIZE_REQUEST_CONTENTS (I
> changed to that, as it seems to be the one which will be used in the
> evolution), "adding" to the previously mentioned issues:
> c) it still happens that the scrollbars in the respective web views are
>    shown - that should be happening

This never happened to me, we could call WebPageProxy::setMainFrameIsScrollable() when WEBKIT_SIZE_REQUEST_CONTENTS is set. But that requires some more changes, because that's mac specific at the moment (even though the implementation is actually cross-platform, so it would be a matter of moving it out of the ifdefs).

> d) when I make the width for the two web views small enough that the
> "looo..ng"
>    word doesn't fit into the allocated width, then change the window height
>    as much as only 3 lines of the text would fit the allocated part, then
>    a busy-loop happens where the view will recalculate whether to show
>    the internal scrollabars or not.

This didn't happen to me either.

> The debug prints loop in the below states
>    with the UI flashing:
> > webkitWebViewBaseSetContentsSize: [198,73] ~> [211,73]
> >    webkitWebViewBaseGetPreferredHeight: contentsSize:[211,73] out:nat:73 min:0 allocated:73
> >    webkitWebViewBaseGetPreferredWidth: contentsSize:[211,73] out:nat:211 min:0 allocated:211
> > webkitWebViewBaseSetContentsSize: [211,73] ~> [198,73]
> >    webkitWebViewBaseGetPreferredHeight: contentsSize:[198,73] out:nat:73 min:73 allocated:63
> >    webkitWebViewBaseGetPreferredWidth: contentsSize:[198,73] out:nat:198 min:0 allocated:211
> 
>    As the busy loop goes through the main loop, it can be avoided when
>    the window size is changed again. I'm pretty sure that this is caused by
>    the extra scrollbars inside the top web view, thus when the a)/c) is
>    addressed the d) might be as well. 
> 
> I cannot reproduce the never-shrinking issue with this patch and the
> WEBKIT_SIZE_REQUEST_CONTENTS being used.

I didn't always happen, it depended on how fast/slow you made the allocated size smaller.
 
> Regarding the get_preferred_width/height being called or not, it seems to me
> that it was the case only when the view couldn't change its size, thus in my
> case only if the given width for the two web views was not enough to be able
> to show the "looo...ng" word. That is, the width of the "looo...ng" word
> defines the width for both web views, which is correct, from my point of
> view.

I made some tests, also enabled the WebPageProxy::setMainFrameIsScrollable() as you suggested, together with some other tweaks, and it is better, but still not the best. Especially, when I give the content more width, then WebCore (I guess) still reports the height which had been allocated when the width was smaller. I can share my test patch with you.

What I'd like to achieve, is to pack some views vertically

   +-----------+
   | view1     |
   |           |
   +-----------+
   | widget    |
   +-----------+
   | view2     |
   |           |
   +-----------+
   | view3     |
   +-----------+

each having different height for the given width. Thus the width is the only common part for all of them, though even there I can imagine that some views will be wider and some smaller. If the content width of the view is bigger than the allocated width of the container, then a horizontal scrollbar will be shown in the parent's scrollable window. Similarly with the summed height of all the vertically packet widgets.

The problem as I see it right now is that the WebCore's (?) Frame/Page content height is influenced by the widget's allocated height and it doesn't shrink even if I make the width enough to have the text on one line only. That is, I want to compute the real minimum content height for a particular width and set this as a height request for the WebView widget. Similarly with width, either I'll use the width of the container (max width of all the packed widgets in the column) or the WebView will change its width based on the real content size, not the max(allocated.width,content.width()) (the same applies for height). What is used is perfectly controlled by the widget's properties hexpand, vexpand, halign, valign, thus no need to do anything in this regard on the webkit side, the only issue is the computation of the real content size.

I even enabled WebCore::Frame::preferredHeight() for everyone, but it doesn't work, it still uses that "max(allocated.height, real_content.height())" formula.

A function which would compute expected content height for given width would be very handful in this case, then this could be used as min/natural size in the get_preferred function(s).

Created attachment 327793 [details]
finished wk.patch

for webkitgtk;

I managed to finish the patch for the webkitgtk, which makes the allocation-based-on-the-content-size working fine in the test application. I didn't spot any busy loops with flickering, no extra scrollbars, nothing. Just perfectly working for my (Evolution) needs.

I removed the ContentWidth/ContentHeight options, there were confusing and (possibly [1]) non-working, thus the current enum has only two options, Default and Content. I tried to express what each of them does in the comments.

[1] the setMinimumLayoutSize() also computes height based on the given width and turns off auto-sizing when the width is 0, which would make the Height (or Width) size request non-working. The new enum consistently reflects this.

Carlos, I've a sad news for you, after some work on the Evolution side with a patched WebKitGTK I realized that the evolution cannot use it. Not only because the WebKitGTK+ doesn't have necessary bits [1][2], but even with then it would mean to maintain in the Evolution a code which would mimic what the WebKitGTK+ does on its own when we left it scrolling. notably the "scroll to the position where the user aims (using Tab key, arrows (with or without caret mode), selection with a mouse, typing into an Input element - all these require the view to be properly scrolled, thus the user sees what he/she does. Another part is searching inside the message for a text occurrence - again something what can the WebKitGTK+ do for free, but with the split EMailDisplay into several widgets (and WebView-s) the searching over all internal WebView-s is just too much code to maintain). I'm afraid that the extra code in the evolution (more or less duplicated from the webkit) would mean too much risk that it'll break unnoticed in some future release of the WebKit or even gtk+ itself (the GtkContainer's "scroll_on_focus" implemented by gtk_container_set_focus_hadjustment() is another nightmare), thus it'll be vital for both WebKitGTK+ and Evolution to not maintain a code which will be pretty easy to break.

I'm sorry I wasted your time on this, I should realize all the issues much sooner than this far.

I'm going to close the [1] too, the same as this bug report. I'll use a different approach in the Evolution, simpler than all the tweaks I initially wanted to have on both WebKitGTK+ and Evolution sides.

[1] https://bugs.webkit.org/show_bug.cgi?id=157790
[2] https://bugs.webkit.org/show_bug.cgi?id=157793