Bug 85843 – [GTK] ENABLE_IFRAME_SEAMLESS support

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Philippe Normand]

Since r116356 seamless frames support is optional and disabled in GTK. So the following tests fail:

Regressions: Unexpected text diff mismatch : (10)
  fast/frames/seamless/seamless-form-get-named.html = TEXT
  fast/frames/seamless/seamless-form-get.html = TEXT
  fast/frames/seamless/seamless-form-post-named.html = TEXT
  fast/frames/seamless/seamless-form-post.html = TEXT
  fast/frames/seamless/seamless-hyperlink-named.html = TEXT
  fast/frames/seamless/seamless-hyperlink.html = TEXT
  fast/frames/seamless/seamless-window-location-href.html = TEXT
  fast/frames/seamless/seamless-window-location-sandbox.html = TEXT
  fast/frames/seamless/seamless-window-location.html = TEXT
  fast/frames/seamless/seamless-window-open.html = TEXT

I'll flag those in test_expectations for now but I think we should simply enable the feature in GTK. Any thoughts, Martin?

[Martin Robinson]

Adam, do you think it's safe to start shipping this feature in unstable releases and then in a stable release in about 6 months?

[Adam Barth]

> Adam, do you think it's safe to start shipping this feature in unstable releases and then in a stable release in about 6 months?

Yes.  Eric is still landing some pieces of the feature, but six months is plenty of time to bake.

[Martin Robinson]

Okay. I'd say it's quite safe to turn this feature on. it carries a lot less risk than some of our API developer facing features and we'll be able to test it more thoroughly by turning it on now.

[Zan Dobersek]

Created attachment 141195 [details]
Patch

[Zan Dobersek]

(In reply to comment #4)
> Created an attachment (id=141195) [details]
> Patch

This patch adds a configuration option in configure.ac to enable this feature, but is disabled by default. The feature is enabled by default in build-webkit, though.

I can however enable this feature as well in configure.ac as the comment #3 hints.

[Martin Robinson]

Comment on attachment 141195 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141195&action=review

> Tools/Scripts/webkitperl/FeatureList.pm:196
> +      define => "ENABLE_ICONDATABASE", default => isGtk(), value => \$iframeSeamlessSupport },

Looks like the define is wrong here. It should probably be ENABLE_IFRAME_SEAMLESS.

[Zan Dobersek]

(In reply to comment #6)
> (From update of attachment 141195 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141195&action=review
> 
> > Tools/Scripts/webkitperl/FeatureList.pm:196
> > +      define => "ENABLE_ICONDATABASE", default => isGtk(), value => \$iframeSeamlessSupport },
> 
> Looks like the define is wrong here. It should probably be ENABLE_IFRAME_SEAMLESS.

Caught red-handed :>

A proper patch incoming.

[Zan Dobersek]

Created attachment 141209 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 141209 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141209&action=review

> Tools/Scripts/webkitperl/FeatureList.pm:196
> +      define => "ENABLE_IFRAME_SEAMLESS", default => isGtk(), value => \$iframeSeamlessSupport },

should still be default => 1

> configure.ac:742
> +              [],[enable_iframe_seamless="no"])

I stillt hink this should defatult to yes.

[Zan Dobersek]

Comment on attachment 141209 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141209&action=review

>> Tools/Scripts/webkitperl/FeatureList.pm:196
>> +      define => "ENABLE_IFRAME_SEAMLESS", default => isGtk(), value => \$iframeSeamlessSupport },
> 
> should still be default => 1

Given that Mac port showed some desire for an option to ship with this feature disabled, I guess this shouldn't affect them?

>> configure.ac:742
>> +              [],[enable_iframe_seamless="no"])
> 
> I stillt hink this should defatult to yes.

Figures.

[Zan Dobersek]

Created attachment 141213 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 141213 [details]
Patch

build-webkit option defaults are wildly inaccurate until I fix bug 85456 anyway. :)  If you're interested in this topic, we still need someone to split the Gtk feature defaults out into some easily autogenerated file to fix that bug!

[WebKit Review Bot]

Comment on attachment 141213 [details]
Patch

Clearing flags on attachment: 141213

Committed r116679: <http://trac.webkit.org/changeset/116679>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.