Bug 108314 – [BlackBerry] Enable non-video element enter/exit fullscreen.

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Mary Wu]

Webkit support non-video element (like div) to enter/exit fullscreen. We should pass this capability.

[Mary Wu]

Created attachment 185452 [details]
Patch

[Antonio Gomes]

Comment on attachment 185452 [details]
Patch

Why not removing the whole code path that is dead?

'm_fullscreenVideoNode' is this name (containing "video") still accurate?

[Mary Wu]

(In reply to comment #2)
> (From update of attachment 185452 [details])

Thanks for comments, Antonio.

> Why not removing the whole code path that is dead?

Will track it in separate PR.
> 
> 'm_fullscreenVideoNode' is this name (containing "video") still accurate?

hehe, yes, "Video" no more suitable here.

[Mary Wu]

Created attachment 186749 [details]
Patch

[Mary Wu]

> > Why not removing the whole code path that is dead?

BTW, which code path, could you give more info? thanks

[Antonio Gomes]

Comment on attachment 186749 [details]
Patch

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

- we have now another inconsistency: fullscreen x fullScreen. We could address it as well in a follow up.

> Source/WebKit/blackberry/Api/WebPage.cpp:5122
> +    Element* element = static_cast<Element*>(d->m_fullscreenNode.get());

because you cast, please call 'm_fullscreenNode->isElement())

[Antonio Gomes]

(In reply to comment #5)
> > > Why not removing the whole code path that is dead?
> 
> BTW, which code path, could you give more info? thanks

the whole "enterFullscreenForNode" is not obsolete. IIRC it was playbook codepath. Not with FULLSCREEN_API, it got obsolete.

Also WebSettings::videoCapable seems obsolete as well.

[Mary Wu]

(In reply to comment #6)
> (From update of attachment 186749 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186749&action=review
> 
> - we have now another inconsistency: fullscreen x fullScreen. We could address it as well in a follow up.
> 
I also noticed that, didn't touch it since not related to the bug :) should address it in follow up.

> > Source/WebKit/blackberry/Api/WebPage.cpp:5122
> > +    Element* element = static_cast<Element*>(d->m_fullscreenNode.get());
> 
> because you cast, please call 'm_fullscreenNode->isElement())

yes, will call toElement()

[Mary Wu]

(In reply to comment #7)
> (In reply to comment #5)
> > > > Why not removing the whole code path that is dead?
> > 
> > BTW, which code path, could you give more info? thanks
> 
> the whole "enterFullscreenForNode" is not obsolete. IIRC it was playbook codepath. Not with FULLSCREEN_API, it got obsolete.
> 
> Also WebSettings::videoCapable seems obsolete as well.

I noticed that, Not sure if other app would use that part in future. Will consult Max. Thanks

[Mary Wu]

Created attachment 186995 [details]
Patch

[WebKit Review Bot]

Comment on attachment 186995 [details]
Patch

Rejecting attachment 186995 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 186995, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
ceeded at 5787 (offset 35 lines).
Hunk #8 succeeded at 5809 (offset 35 lines).
Hunk #9 succeeded at 5846 (offset 40 lines).
Hunk #10 succeeded at 5854 (offset 40 lines).
Hunk #11 succeeded at 5873 (offset 40 lines).
2 out of 11 hunks FAILED -- saving rejects to file Source/WebKit/blackberry/Api/WebPage.cpp.rej
patching file Source/WebKit/blackberry/Api/WebPage_p.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16430148

[WebKit Review Bot]

Comment on attachment 186995 [details]
Patch

Rejecting attachment 186995 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 186995, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
ceeded at 5787 (offset 35 lines).
Hunk #8 succeeded at 5809 (offset 35 lines).
Hunk #9 succeeded at 5846 (offset 40 lines).
Hunk #10 succeeded at 5854 (offset 40 lines).
Hunk #11 succeeded at 5873 (offset 40 lines).
2 out of 11 hunks FAILED -- saving rejects to file Source/WebKit/blackberry/Api/WebPage.cpp.rej
patching file Source/WebKit/blackberry/Api/WebPage_p.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16438151

[Mary Wu]

Comment on attachment 186995 [details]
Patch

need to rebase to update code.

[Mary Wu]

Created attachment 187213 [details]
Patch

[WebKit Review Bot]

Comment on attachment 187213 [details]
Patch

Clearing flags on attachment: 187213

Committed r142220: <http://trac.webkit.org/changeset/142220>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.