WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#15561 closed defect (bug) (fixed)

Press This is broken

Reported by: westi Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Press This Keywords: has-patch
Focuses: Cc:

Description (last modified by westi)

  • When using Press This, you are unable to add tags to your post.
  • Link helper not working

...

Attachments (12)

15561.js-error.diff (889 bytes) - added by duck_ 4 years ago.
15561.001.diff (1.2 KB) - added by duck_ 4 years ago.
garyc40-15561.patch (937 bytes) - added by garyc40 4 years ago.
link dialog style fix
garyc40-15561-rev2.patch (1.1 KB) - added by garyc40 4 years ago.
move styles to wplink.css
garyc40-15561-rev3.patch (1.1 KB) - added by garyc40 4 years ago.
use full font stack
15561.rearranged.diff (1.4 KB) - added by nacin 4 years ago.
15886-styling-white-background.diff (791 bytes) - added by dd32 4 years ago.
garyc40-15561-rev4.patch (1.6 KB) - added by garyc40 4 years ago.
wplink patch refreshed
garyc40-15561-rev5.patch (1.2 KB) - added by garyc40 4 years ago.
my bad, refreshed again
garyc40-15561-rev6.patch (821 bytes) - added by garyc40 4 years ago.
reverted change in wp-admin.css
garyc40-15561-rev7.patch (853 bytes) - added by ocean90 4 years ago.
Set font-size: 12px.
15561.quickdirty.patch (661 bytes) - added by ocean90 4 years ago.

Download all attachments as: .zip

Change History (46)

comment:1 westi4 years ago

  • Description modified (diff)

duck_4 years ago

comment:2 duck_4 years ago

15561.js-error.diff fixes an undefined error which blocks AJAX tags from working. Nothing on the link button yet.

duck_4 years ago

comment:3 follow-up: duck_4 years ago

15561.001.diff fixes both the tags and the link button. Though some more CSS styles for the link dialog need to be included.

Could press-this.php not make use of admin-header.php to help stop these kind of problems in the future?

comment:4 ryan4 years ago

(In [16571]) Press This fixes. Props duck_. see #15561

comment:5 in reply to: ↑ 3 duck_4 years ago

Replying to duck_:

15561.001.diff fixes both the tags and the link button. Though some more CSS styles for the link dialog need to be included.

Okay, one of the missing CSS style sheets for the link dialog is the global style sheet (for example getting rid of the bullets in the search list). But I'll leave that for now, I don't know if this will introduce conflicts with other press-this styles.

Could press-this.php not make use of admin-header.php to help stop these kind of problems in the future?

Answer no. This would include the large admin header, favourite actions, etc. However the current approach for Press This seems a bit annoying for maintenance :(

comment:6 scribu4 years ago

re maintenance: #14650

comment:7 ryan4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

comment:8 duck_4 years ago

  • Priority changed from high to normal
  • Resolution fixed deleted
  • Severity changed from blocker to normal
  • Status changed from closed to reopened

Still some styling issues.

comment:9 ocean904 years ago

#15758 closed as a dup.

garyc404 years ago

link dialog style fix

comment:10 garyc404 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

comment:11 nacin4 years ago

I would rather see these link dialog reset fixes go into wplink.dev.css directly, that way we don't need to maintain two different versions of the link dialog CSS.

garyc404 years ago

move styles to wplink.css

comment:12 follow-up: nacin4 years ago

font-family needs the whole stack. Looks alright otherwise, but I am guessing some of these rules can be merged into the rest of the styles. For example, the toggle-arrow rules can go into toggle-arrow directly.

garyc404 years ago

use full font stack

comment:13 in reply to: ↑ 12 garyc404 years ago

Replying to nacin:

I am guessing some of these rules can be merged into the rest of the styles. For example, the toggle-arrow rules can go into toggle-arrow directly.

Hmm, don't fully understand your suggestion. I'm guessing you mean using rules that are defined elsewhere instead of porting those rules from other stylesheets to wplink.css?

The problem with press this is that it has its own stylesheet (press-this.css) and doesn't inherit from base css files (such as global.css or wp-admin.css). As a result, lots of code duplication there. But since including these global and wp-admin stylesheets would be costly in terms of overhead, I don't see any other solution than copypasta. Splitting up wp-admin.css into smaller components that press-this can inherit might work, although that would be a bit unwieldy.

comment:14 nacin4 years ago

Sorry, I figured that some of those selectors you added to the top of the file were repeated later in the file, and that some of those properties could be moved and thus declaration blocks combined.

I now see that's not necessarily the case.

Posting a patch that shifts things around into a slightly more logical order (doesn't change any CSS), and asking koopersmith to review it. Thanks for the patch.

nacin4 years ago

comment:15 nacin4 years ago

Still having issues with long titles.

comment:16 follow-up: dd324 years ago

Closed #15886 as a dup of this:

Press This's Category metabox's tabs have a White background on the <a>.


This background is coming from `body.press-this .tabs a, body.press-this
.tabs a:hover `. The .tabs class is not used elsewhere and appears to be a
left over fragment from something else.


Attached patch removes the stylings for the .tab class, and brings the
styling inline with the standard editor by applying a #333 link colour
whilst the tab is selected.

comment:17 in reply to: ↑ 16 duck_4 years ago

Replying to dd32:

Closed #15886 as a dup of this:

Press This's Category metabox's tabs have a White background on the <a>.


This background is coming from `body.press-this .tabs a, body.press-this
.tabs a:hover `. The .tabs class is not used elsewhere and appears to be a
left over fragment from something else.


Attached patch removes the stylings for the .tab class, and brings the
styling inline with the standard editor by applying a #333 link colour
whilst the tab is selected.

I actually remember doing this ages ago. See #12809, [14149] and [14150] (last one fix for colors-classic too). Looks like it might have been accidentally reverted in the next commit against colors-fresh.dev.css for nav menus.

comment:18 dd324 years ago

I actually remember doing this ages ago.

Me too, I had however, thought it was the normal admin category metabox :)

comment:19 ryan4 years ago

Per IRC discussion, all patches are good for commit. This ticket will be left open for further tweaks.

comment:20 ryan4 years ago

wplink patches need refresh.

garyc404 years ago

wplink patch refreshed

comment:21 nacin4 years ago

Latest wplink patch needs refresh again.

garyc404 years ago

my bad, refreshed again

garyc404 years ago

reverted change in wp-admin.css

ocean904 years ago

Set font-size: 12px.

comment:22 nacin4 years ago

(In [17131]) Fix some tab CSS in Press This. props dd32. Also adjust menu bits to default to gray, and then add rules to blue. see #15561.

comment:23 dremeda4 years ago

Seems good now Nacin.

comment:25 nacin4 years ago

Great. All patches here are committed.

Press This needs final RTL and IE6 checks. Internal linking in both Press This and post-new should be checked for long titles as well (including in IE6, RTL, and both IE6 and RTL).

Please check off what you've tested.

comment:26 dremeda4 years ago

I've checked latest Mac browsers (FF/Chrome/Safari/Opera).

comment:27 nacin4 years ago

Final checks for internal linking (IE6, RTL, long titles) need to be reported to #15739.

comment:28 nacin4 years ago

(In [17135]) More CSS reset for wplink. Give the dialog title a specific line height. see #15561.

comment:30 ocean904 years ago

Long titles and the internal linking box are fine in IE6/7 and Opera.

IE6-8 problem: In Press This the internal linking box is empty: http://grab.by/842L

comment:31 nacin4 years ago

Any inkling as to why? I'll try to fire IE8 up on a machine.

comment:32 ocean904 years ago

  • Keywords needs-patch added; has-patch needs-testing removed

It's a no-go, that we add the HTML code for the dialog (wp_link_dialog()) in the <head> tag. This could be the problem in IE, but I can't test more yet.

ocean904 years ago

comment:33 ocean904 years ago

  • Keywords has-patch added; needs-patch removed

15561.quickdirty.patch will fix the bug. Now it works in IE6,7 and 8 too. Related changeset r16571. Patch is quick&dirty, don't have the time. Merry christmas. :-)

comment:34 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17140]) Delay wp_tiny_mce_preload_dialogs to the footer, as occurs in admin-header. props ocean90, fixes #15561.

Note: See TracTickets for help on using tickets.