r101674 added WebCore settings to enable and disable subtitle, caption, and description text tracks. Add web preferences for them.
<rdar://problem/10522425>
Created attachment 117732 [details] Proposed patch
Comment on attachment 117732 [details] Proposed patch Attachment 117732 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10733219
(In reply to comment #3) > (From update of attachment 117732 [details]) > Attachment 117732 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/10733219 Blah blah, blah blah, blah blah.
Created attachment 117738 [details] Updated patch
Created attachment 117757 [details] Updated again Don't export the settings from the Mac version of WebCore, it does't build with ENABLE_VIDEO_TRACK defined.
Comment on attachment 117757 [details] Updated again Attachment 117757 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10690474 New failing tests: svg/custom/linking-uri-01-b.svg
Comment on attachment 117757 [details] Updated again Are you intentionally not exposing these settings via WebKit1 on Windows?
Note that I reviewed this before I saw Jeff’s comment about WebKit1 on Windows.
(In reply to comment #9) > Note that I reviewed this before I saw Jeff’s comment about WebKit1 on Windows. Thanks, I will fix that oversight before committing.
Comment on attachment 117757 [details] Updated again View in context: https://bugs.webkit.org/attachment.cgi?id=117757&action=review > Source/WebCore/page/Settings.cpp:854 > +#if ENABLE(VIDEO_TRACK) > +void Settings::setShouldDisplaySubtitles(bool flag) > +{ > + m_shouldDisplaySubtitles = flag; > +} > + > +void Settings::setShouldDisplayCaptions(bool flag) > +{ > + m_shouldDisplayCaptions = flag; > +} > + > +void Settings::setShouldDisplayTextDescriptions(bool flag) > +{ > + m_shouldDisplayTextDescriptions = flag; > +} > +#endif Why this change? You moved these into the cpp file, but they are the same as what was in the .h file?
(In reply to comment #11) > (From update of attachment 117757 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=117757&action=review > > > Source/WebCore/page/Settings.cpp:854 > > +#if ENABLE(VIDEO_TRACK) > > +void Settings::setShouldDisplaySubtitles(bool flag) > > +{ > > + m_shouldDisplaySubtitles = flag; > > +} > > + > > +void Settings::setShouldDisplayCaptions(bool flag) > > +{ > > + m_shouldDisplayCaptions = flag; > > +} > > + > > +void Settings::setShouldDisplayTextDescriptions(bool flag) > > +{ > > + m_shouldDisplayTextDescriptions = flag; > > +} > > +#endif > > Why this change? You moved these into the cpp file, but they are the same as what was in the .h file? The functions were inlined when they were in the .h file (noted in the ChangeLog), so the symbols couldn't be exported from WebCore . I can move them back if you would rather.
Created attachment 117815 [details] Updated patch Updated to include Windows so the EWS bot can check it out.
The new patch doesn’t have review? set. (If it did, I would review+ it.)
Created attachment 117818 [details] Yet another patch.
(In reply to comment #14) > The new patch doesn’t have review? set. (If it did, I would review+ it.) I remembered this time, and this one should actually compile!
Comment on attachment 117818 [details] Yet another patch. r+ if you’re right!
Comment on attachment 117818 [details] Yet another patch. Clearing flags on attachment: 117818 Committed r101977: <http://trac.webkit.org/changeset/101977>
All reviewed patches have been landed. Closing bug.
(In reply to comment #12) => The functions were inlined when they were in the .h file (noted in the ChangeLog), so the symbols couldn't be exported from WebCore. For functions in the header with the inline keyword, you shouldn’t need to export them from WebCore. So I think that would have been a better way to god.
to go.
(In reply to comment #20) > (In reply to comment #12) > => The functions were inlined when they were in the .h file (noted in the ChangeLog), so the symbols couldn't be exported from WebCore. > > For functions in the header with the inline keyword, you shouldn’t need to export them from WebCore. So I think that would have been a better way to go. Thanks, I will change it in a follow up patch.
(In reply to comment #22) > (In reply to comment #20) > > (In reply to comment #12) > > => The functions were inlined when they were in the .h file (noted in the ChangeLog), so the symbols couldn't be exported from WebCore. > > > > For functions in the header with the inline keyword, you shouldn’t need to export them from WebCore. So I think that would have been a better way to go. > > Thanks, I will change it in a follow up patch. https://bugs.webkit.org/show_bug.cgi?id=73879 is for the revert.