Make WordPress Core

Opened 16 years ago

Closed 16 years ago

#7066 closed enhancement (fixed)

Press This major enhancements, ajax, tabs, photo loading

Reported by: noel's profile noel Owned by: noel's profile noel
Milestone: 2.6 Priority: high
Severity: normal Version: 2.6
Component: Administration Keywords:
Focuses: Cc:

Description

Major fixes and enhancments to the javascript, ajax loading and removal of jquery-ui-tabs.

Attachments (3)

press-this-may-30-2am.diff (22.5 KB) - added by noel 16 years ago.
press this fixes as of may 30th at 2:20am from Noel
press-this-6-3-08-8pm.diff (32.5 KB) - added by noel 16 years ago.
Press This + Sideloader
press-this-css.patch (961 bytes) - added by azaozz 16 years ago.
Fix for the tag suggest drop-down list

Download all attachments as: .zip

Change History (19)

@noel
16 years ago

press this fixes as of may 30th at 2:20am from Noel

#1 @mdawaffe
16 years ago

The one other (that I know of) place where we use ui.tabs is in the Add Category interface on the Write screen. It's proven to give a good amount of flexibility for plugins which change that interface around.

What prompted the switch here? Is ui.tabs too bloated? The code in the patch looks like a plugin author could hack in new tabs/functionality, but I'm not sure. Do you think that it's extensible? (If that's even a concern.)

#2 @ryan
16 years ago

Indeed, ui.tabs makes it easy for plugins to add new tabs. That's one of the reasons we used it. And because I like to maintain as little JS as possible. :-)

PHP_SELF looks like it is echoed raw in a couple places. That's a possible XSS problem.

#3 @noel
16 years ago

I opted to not use ui.tabs after talking with Matt and Andrew about it. They both felt that it presented little value and the plugin would be better off with minimal javascript.

Do we really want the bookmarklet to be extensible? I'd opt for less code than something that allowed a ton of manipulation. The way ui.tabs behaved meant the plugin needed to do more work than necessary - each screen isn't very different from the last, so just putting the differences into necessary requests seems to make more sense.

#4 @noel
16 years ago

Since you have to be logged in, to use the script, is PHP_SELF an issue? If not, what is the proposed solution?

#5 @noel
16 years ago

  • Cc noel@… added

#6 @ryan
16 years ago

I think you can just run PHP_SELF through clean_url(). The current patch no longer applies cleanly; can I trouble you to do a new patch off of latest trunk and try out running PHP_SELF through clean_url()?

#7 @noel
16 years ago

Not a problem. I've almost got a sideloader polished up, so I'll post the diff when that's done.

Wondering where we stand on the ui.tabs.

After remembering why I wanted to ditch them, one of the many reasons was that the current ajax tag code interferes with ui.tabs.js. (That is aside from the speed increase seen after removing the tabs js.)

@noel
16 years ago

Press This + Sideloader

#8 @noel
16 years ago

The newest diff should address all the problems except for re-implementing jquery-ui-tabs since it interferes with the ajax tag code. Support for vimeo was added and lots of bugs were fixed.

#9 @ryan
16 years ago

(In [8048]) Press This enhancement from noel. see #7066

#10 @azaozz
16 years ago

I think there were two reasons not to use ui.tabs here: it doesn't play well with TinyMCE and the tag suggest, and it's slower compared to only loading the differences between the tabs and leaving the editor, the tags field and the categories in place as in the current version (also discussed in #6998).

To make the press-this extensible, we could add a hook in the tabs that would reload the whole page inside the popup and display content supplied by a plugin, similarly to how the admin submenu/plugins settings pages work. That way all scripts would be unloaded correctly and initiated again when needed.

Currently the add image functionality would need some refining. In most browsers it is possible to drag the small preview images to the editor and they will be inserted but with the wrong settings. I think many users would try doing that. There are ondrag, ondrop, etc. js events but the support for them varies a lot between the browsers. Perhaps we could try disabling this. Another option is to make a TinyMCE plugin that would handle it properly in the browsers that support it, and let it insert the html tags.

Also clicking on a thumbnail in the images field shows second preview in Thickbox and at the same time inserts the image into the editor. Perhaps it would be more user friendly to either have "OK" and "Cancel" on the Thickbox preview or not show it at all, as it's a bit confusing.

Don't think the "extended_valid_elements" setting is needed in TinyMCE. Most of it repeats the default rules and adds some non XHTML attributes. For handling video we could try including the "Media" plugin (as in the default editor settings). It has support for almost all current video formats and we don't need to include the button on the toolbar for it to work. It would produce the object/embed tags when given the url to a video file.

#11 @ryan
16 years ago

Extensibility is low priority. If we can do it without adding too much complexity or slowing things down, fine. Press This should be fast and light, first and foremost.

Maybe disable image dragging for now?

I like the image preview, but Select or Cancel would indeed be nice. It's not apparent right now that it does both preview and select.

Right now everything is working pretty well for me except for tag suggest. The suggested tags pop up over the title rather than below the tag input.

#12 @noel
16 years ago

Also, there is a problem with ajax requests and urls with long query strings. I've almost got that sorted, but wanted to note it, so it doesn't slip my mind.

#13 @noel
16 years ago

  • Owner changed from ryan to noel

@azaozz
16 years ago

Fix for the tag suggest drop-down list

#14 @azaozz
16 years ago

Since Press This doesn't load wp-admin.css, a few classes have to be copied to press-this.css, so the tag suggest works properly.

#15 @ryan
16 years ago

(In [8062]) Fix the tag suggest drop-down list for Press This. Props azaozz. see #7066

#16 @ryan
16 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.