Download & Extend

Back out browser.inc

Project:Drupal core
Version:7.x-dev
Component:browser system
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (fixed)
Issue tags:API change

Issue Summary

This issue is a repost of comment #101 in #340283: Abstract SimpleTest browser in to its own object. I am opening a separate issue for discussing this.

---

So far, no code in core uses the new browser. While trying to convert drupal_http_request() to a wrapper around the browser (#553280: Integrate browser.inc and drupal_http_request()), I noticed that it lacked several methods in order to provide the same functionality as drupal_http_request(). I think this would also be the case when replacing the Simpletest browser in #553278: Replace DrupalWebTestCase browser with browser.inc. In order to fix these, modifying/extending the browser API is necessary.

There is just one week until the API freeze on October 15, but it seems like there is still a lot to do. If we are not able to get at least #553278: Replace DrupalWebTestCase browser with browser.inc committed in time for the freeze (not that fixing that bug itself would constitute an API change, but it will give us an understanding of the necessary changes to the browser API), would it be better to remove browser.inc for D7?

Don't get me wrong — I like the idea about a separate browser class and I appreciate the work that has gone into this, but I'd rather not see D7 ship with three different HTTP implementations (drupal_http_request(), DWTC and browser.inc) because there is no time to consolidate the code.

Comments

#1

I'd be fine wish pushing back to D8 and properly getting this integrated with other HTTP-request type code. I don't see it happening anytime in the next week. :(

Sad +1

#2

Status:active» needs review

Don't know if this is too late for D7, but here is a patch.

AttachmentSizeStatusTest resultOperations
remove-browser-1.patch39.07 KBIdleFailed: Failed to apply patch.View details

#3

Status:needs review» reviewed & tested by the community

It sounds like this was committed in the hopes of follow-on implementations. Those will come, but not for D7

#4

Status:reviewed & tested by the community» needs work

The last submitted patch failed testing.

#5

Status:needs work» needs review
AttachmentSizeStatusTest resultOperations
remove-browser-2.patch2.47 KBIdleFailed: 14656 passes, 13 fails, 13 exceptionsView details

#6

Status:needs review» needs work

The last submitted patch failed testing.

#7

Status:needs work» needs review

The last patch was incomplete.

AttachmentSizeStatusTest resultOperations
remove-browser-3.patch39.22 KBIdlePassed: 14680 passes, 0 fails, 0 exceptionsView details

#8

Status:needs review» fixed

I spoke with Jimmy about this, and he agreed he'd like to work on improving the browser over the D7 release cycle. He's created http://drupal.org/project/browser for that, which wraps browser.inc into a module.

Therefore, committed to HEAD.

#9

Status:fixed» closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nobody click here