Issues with ElggFile::detectMimeType() #9010

Closed
hypeJunction opened this Issue Oct 8, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Contributor

hypeJunction commented Oct 8, 2015

I am seeing strange behaviour ever since we introduced #8889.

When calling ElggFile::detectMimeType() from a contcrete class instance (other than ElggFile), $this is considered set and refers to the concrete instance. As a result the script is WSODing, as getFilenameOnFilestore does not exist for that concrete class.

We should deprecate the static use, and add another static method. In the meantime, we should perhaps make sure that $this is an instance of ElggFile

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Oct 8, 2015

Member

When calling ElggFile::detectMimeType() from a contcrete class instance (other than ElggFile), $this is considered set

I don't think PHP does this. Yes, that's what's happening. https://3v4l.org/oiHVg They fix it in 7.0, but it's still deprecated.

Member

mrclay commented Oct 8, 2015

When calling ElggFile::detectMimeType() from a contcrete class instance (other than ElggFile), $this is considered set

I don't think PHP does this. Yes, that's what's happening. https://3v4l.org/oiHVg They fix it in 7.0, but it's still deprecated.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Oct 8, 2015

Member

Wait, it does! I read the output wrong

Member

mrclay commented Oct 8, 2015

Wait, it does! I read the output wrong

@mrclay mrclay added the bug label Oct 8, 2015

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Oct 8, 2015

Member

Your suggestion SGTM

Member

mrclay commented Oct 8, 2015

Your suggestion SGTM

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Oct 10, 2015

Contributor

What are you thoughts on deprecation? Should we just convert to:


public static function detectMimeType($file, $default = null) {
    if (!isset($file)) {
        // deprecation notice
    }
}
Contributor

hypeJunction commented Oct 10, 2015

What are you thoughts on deprecation? Should we just convert to:


public static function detectMimeType($file, $default = null) {
    if (!isset($file)) {
        // deprecation notice
    }
}
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Oct 16, 2015

Member

Fixed by #9015

Member

mrclay commented Oct 16, 2015

Fixed by #9015

@mrclay mrclay closed this Oct 16, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment