WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#7066 closed enhancement (fixed)

Press This major enhancements, ajax, tabs, photo loading

Reported by: noel Owned by: 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 6 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 6 years ago.
Press This + Sideloader
press-this-css.patch (961 bytes) - added by azaozz 6 years ago.
Fix for the tag suggest drop-down list

Download all attachments as: .zip

Change History (19)

noel6 years ago

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

comment:1 mdawaffe6 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.)

comment:2 ryan6 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.

comment:3 noel6 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.

comment:4 noel6 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?

comment:5 noel6 years ago

  • Cc noel@… added

comment:6 ryan6 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()?

comment:7 noel6 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.)

noel6 years ago

Press This + Sideloader

comment:8 noel6 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.

comment:9 ryan6 years ago

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

comment:10 azaozz6 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.

comment:11 ryan6 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.

comment:12 noel6 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.

comment:13 noel6 years ago

  • Owner changed from ryan to noel

azaozz6 years ago

Fix for the tag suggest drop-down list

comment:14 azaozz6 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.

comment:15 ryan6 years ago

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

comment:16 ryan6 years ago

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