Page MenuHomePhabricator

Adding/removing to watchlist when new user causes window to reopen
Closed, ResolvedPublic


Visit http://localhost:8888/w/index.php?title=Headings&article_action=add_to_collection&mobileaction=beta
Add/remove the page from your Watchlist
The content overlay will confusingly reopen
Only happens for Watchlist...

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task to In Analysis on the Gather Sprint Enwiki board.
Jdlrobson added a subscriber: Jdlrobson.

Change 203842 had a related patch set uploaded (by Jdlrobson):
Prevent auto-reopening of collection overlay

Change 203842 merged by jenkins-bot:
Prevent auto-reopening of collection overlay


Seriously though, shouldn't the options object always be the same as this.options? What's going on?

Forgive my stupidity, but I still don't quite understand what's going on here. Context: I came here by way of the byref patch that you submitted.

@phuedx no problems - it's really subtle and I'm confused now trying to explain it. So in render we were basically cloning options and then adding the newly passed parameters.

if options was { a: 1 }

foo.render( { foo: bar } ) would render the template with foo and a
foo.render() would render the template with simply a, regardless of the previous execution and thus current state of the View
but = bar; foo.render() would render the template with foo and a.
So the change now is that foo.render( { foo: bar } ) overrides existing options.

This became problematic in as postRender needs to changes the current state of CollectionsWatchstar - it deletes an entry from the options parameter - which is sometimes this.options and sometimes an arbitary object. In L163 of that patch we just call render without any arguments which is why we were hitting this problem

@Jdlrobson this is resolved, but appears to have caused a new bug: T97268