Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$response object passed as service argument instead of resolveDispositionHeaderFilename() argument #7

Merged
merged 2 commits into from
Oct 1, 2012

Conversation

inmarelibero
Copy link
Contributor

$request object is now passed through services.xml instead of passing it optionally in the controller as option.
What do you think?

@igorw
Copy link
Owner

igorw commented Oct 1, 2012

I wanted to avoid scoping everything to request, because it's messy. But it's probably a good idea for convenience.

protected $fullFilename;
protected $contentType;
protected $options;

public function __construct($baseDir)
public function __construct($baseDir, $request)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a type hint for Request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, i put a use Symfony\Component\HttpFoundation\Request; in the top, like use Symfony\Component\HttpFoundation\Response;

removed unnecessary check on $response object
@inmarelibero
Copy link
Contributor Author

yes, I found a bit complicate for users to be forced to pass the $request object in the create()function just to avoid the problem in IE, Safari and Konqueror. And that it should be transparent for the user.

@igorw
Copy link
Owner

igorw commented Oct 1, 2012

No, the default behaviour is to send the invalid header which works in all browsers. You need to pass the request to get the valid one.

@igorw
Copy link
Owner

igorw commented Oct 1, 2012

Can you adjust the README?

@inmarelibero
Copy link
Contributor Author

just to make it clear:

  • now users are not required to pass the $request object in the controller as option to the create() function
  • I update the readme writing that support for browsers that are not RFC compliant (IE, safari and konqueror) has been provided

right?

@igorw igorw merged commit 8ab28db into igorw:master Oct 1, 2012
@igorw
Copy link
Owner

igorw commented Oct 1, 2012

I've updated it myself.

Ist gemerged ;-)

@inmarelibero
Copy link
Contributor Author

ok cool. many thanks for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants