[webkit-reviews] review granted: [Bug 29167] REGRESSION(r48064): mint.com loses scrollbars after coming out of edit mode : [Attachment 42636] updateCanHaveScrollbars

[bugzilla-daemon at webkit.org]

mitz at webkit.org has granted Mark Mentovai <mark at chromium.org>'s request for
review:
Bug 29167: REGRESSION(r48064): mint.com loses scrollbars after coming out of
edit mode
https://bugs.webkit.org/show_bug.cgi?id=29167

Attachment 42636: updateCanHaveScrollbars
https://bugs.webkit.org/attachment.cgi?id=42636&action=review

------- Additional Comments from mitz at webkit.org
Thanks for working on this!

I said that I will r+ this patch, but I do have one question and one comment:

>      ScrollbarMode hMode;
>      ScrollbarMode vMode;
> -    scrollbarModes(hMode, vMode);
> +    if (!m_canHaveScrollbars) {
> +	   hMode = ScrollbarAlwaysOff;
> +	   vMode = ScrollbarAlwaysOff;
> +    } else {
> +	   scrollbarModes(hMode, vMode);
> +	   if (hMode == ScrollbarAlwaysOff)
> +	       hMode = ScrollbarAuto;
> +	   if (vMode == ScrollbarAlwaysOff)
> +	       vMode = ScrollbarAuto;
> +    }

Can’t you just use m_canHaveScrollbars to choose unconditionally between
ScrollbarAlwaysOff and ScrollbarAuto?

> -    void setCanHaveScrollbars(bool flag);
> +    virtual void setCanHaveScrollbars(bool flag);

You should remove the parameter name from the declaration.

As promised, r=me.