I can't see why Services should support something as Flash-specific as this, as Services core doesn't do anything that has to do with Flash.

I think services_crossdomain_xml() should be removed from Services core and instead be part of a contrib module of its own or part of a contrib module like Services Accept Origin or AMFPHP.

Things to do:

  • Remove services_crossdomain_xml()
  • Remove services_services_crossdomain_domain_policy()
  • Remove services_xml_output()
  • Remove menu item for crossdomain.xml
  • Anything else?
CommentFileSizeAuthor
#15 1020388_test.patch1.63 KBgdd
#8 services-1020388.patch3.54 KBkylebrowning
#7 services-1020388.patch3.53 KBkylebrowning
#6 services-1020388.patch2.08 KBkylebrowning
#5 services-1020388.patch3.46 KBkylebrowning
#4 services-1020388.patch33 byteskylebrowning

Comments

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented

There is a guy working on a 3.x port of AMFPHP (#926848: Initial port to services 3.x) and I've asked him to comment here.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented

Is this really only useful for Flash? If so I'd agree it should move to AMFPHP, but I thought it was a more generic specification.

tauno’s picture

tauno commented

This is tagged for 7.x, is this for the 6.x branch as well?

I don't see a problem with adding the crossdomain functionality to my amfphp patch. It's still just a patch though and I'm not a maintainer for amfphp, so who know when or if it will be accepted.

kylebrowning’s picture

kylebrowning commented
Status: Active » Needs review
StatusFileSize
new services-1020388.patch33 bytes

Attached patch for 7.x please review.

kylebrowning’s picture

kylebrowning commented
StatusFileSize
new services-1020388.patch3.46 KB

Woops, heres the real patch.

kylebrowning’s picture

kylebrowning commented
StatusFileSize
new services-1020388.patch2.08 KB

Ugh, one more try.

kylebrowning’s picture

kylebrowning commented
StatusFileSize
new services-1020388.patch3.53 KB

Ok, dunno if this is the last one, but I think it is, we also need to update the unit tests to handle the menu modifications.

kylebrowning’s picture

kylebrowning commented
StatusFileSize
new services-1020388.patch3.54 KB

This is failing b/c our test cases are failing for some reason. Definitely seeing the test cases fail locally, so if there are no other objections, id like to commit this patch in.

kylebrowning’s picture

kylebrowning commented
Status: Needs review » Fixed

Fixed in 3.x-dev

voxpelli’s picture

voxpelli commented

We need to make sure that some other modules incorporate this now or that they at least are aware of the removal. Could anyone create feature request issues in the relevant modules pointing them to this issue?

tauno’s picture

tauno commented

Noted on the 6.x-3.x patch I started for AMFPHP: #926848: Initial port to services 3.x

kylebrowning’s picture

kylebrowning commented
Status: Fixed » Needs review

#5: services-1020388.patch queued for re-testing.

kylebrowning’s picture

kylebrowning commented

I re-qued just to see if our committed TEsts are fixed.

Ill close this once i get that information.

THanks!

kylebrowning’s picture

kylebrowning commented
Status: Needs review » Closed (fixed)
gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
commented
StatusFileSize
new 1020388_test.patch1.63 KB

When we committed this, we forgot to remove the test in TestServicesModule.test whic would then fail. I just committed the attached patch to fix this.