Bug 124715

Summary: [GStreamer] Invalid command line error when visiting www.chessbase.com
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: WebKitGTKAssignee: Xabier Rodríguez Calvar <calvaris>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, commit-queue, eric.carlson, glenn, gustavo, jer.noble, menard, mrobinson, pnormand, svillar, vjaquez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.chessbase.com
Bug Depends on: 85994    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Sergio Villar Senin
Reported 2013-11-21 07:08:12 PST
Whenever I visit that page I get a popup window with this error: "gstreamer|1.0|WebKitWebProcess|decodificador text/html|decoder-text/html" because apparently there is a wrong parameter.
Attachments
Patch (1.85 KB, patch)
2013-11-24 09:46 PST, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Patch (1.72 KB, patch)
2013-11-25 09:25 PST, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Patch (5.12 KB, patch)
2013-11-26 09:29 PST, Xabier Rodríguez Calvar 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Xabier Rodríguez Calvar
Comment 1 2013-11-24 09:46:29 PST
Created attachment 217763 [details] Patch HTTP errors were not being handled at the GStreamer WebKit source
Philippe Normand
Comment 2 2013-11-24 23:07:58 PST
Comment on attachment 217763 [details] Patch Hum, please talk to Andres, he has a patch for this code too.
Xabier Rodríguez Calvar
Comment 3 2013-11-25 00:51:03 PST
Comment on attachment 217763 [details] Patch There's a problem with the comment.
Xabier Rodríguez Calvar
Comment 4 2013-11-25 09:25:32 PST
Created attachment 217814 [details] Patch Rebased
Sergio Villar Senin
Comment 5 2013-11-25 09:37:17 PST
Comment on attachment 217814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=217814&action=review I guess this won't be accepted without a test case. > Source/WebCore/ChangeLog:5 > + You used an incorrect bug number to create this patch > Source/WebCore/ChangeLog:7 > + Short explanation missing here. > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:847 > + Don't know a lot about GStreamer but shouldn't this be protected by the locker? Also looks like the error handling code is exactly the same as the one bellow that handles invald partial respones (!= 206). Could it be possible to refactor it in some function or macro?
Xabier Rodríguez Calvar
Comment 6 2013-11-25 09:59:52 PST
(In reply to comment #5) > (From update of attachment 217814 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=217814&action=review > > I guess this won't be accepted without a test case. It is very port specific, writing a test for this and flagging it everywhere that is not gstreamer port feels wrong, specially because this is not a new feature. > > Source/WebCore/ChangeLog:5 > > + > > You used an incorrect bug number to create this patch Roger. > > Source/WebCore/ChangeLog:7 > > + > > Short explanation missing here. Roger. > > Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:847 > > + > > Don't know a lot about GStreamer but shouldn't this be protected by the locker? Well, I considered that, but we would lock, check the response code and then unlock. I don't think it is necessary, though maybe Phil thinks otherwise. > Also looks like the error handling code is exactly the same as the one bellow that handles invald partial respones (!= 206). Could it be possible to refactor it in some function or macro? I though about it but as we have that locker, that I didn't have the intention to use, that is a stack variable, I would have to pass it and I though it wasn't worth, or writing a macro.
Xabier Rodríguez Calvar
Comment 7 2013-11-26 09:29:10 PST
Created attachment 217882 [details] Patch Added test. It doesn't fail either because it doesn't play either though for a different reason.
Xabier Rodríguez Calvar
Comment 8 2013-11-26 09:31:03 PST
(In reply to comment #6) > > I guess this won't be accepted without a test case. > > It is very port specific, writing a test for this and flagging it everywhere that is not gstreamer port feels wrong, specially because this is not a new feature. Now that I think about it, I could use the internals to know the reason of the failure and check if it is the right one, but it seems overkilling to me.
WebKit Commit Bot
Comment 9 2013-11-27 02:45:18 PST
Comment on attachment 217882 [details] Patch Clearing flags on attachment: 217882 Committed r159810: <http://trac.webkit.org/changeset/159810>
WebKit Commit Bot
Comment 10 2013-11-27 02:45:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.