WordPress.org

Make WordPress Core

Opened 5 years ago

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

Download all attachments as: .zip

Change History (46)

#1 @westi
5 years ago

  • Description modified (diff)

@duck_
5 years ago

#2 @duck_
5 years ago

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

@duck_
5 years ago

#3 follow-up: @duck_
5 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?

#4 @ryan
5 years ago

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

#5 in reply to: ↑ 3 @duck_
5 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 :(

#6 @scribu
5 years ago

re maintenance: #14650

#7 @ryan
5 years ago

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

#8 @duck_
5 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.

#9 @ocean90
5 years ago

#15758 closed as a dup.

@garyc40
5 years ago

link dialog style fix

#10 @garyc40
5 years ago

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

#11 @nacin
5 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.

@garyc40
5 years ago

move styles to wplink.css

#12 follow-up: @nacin
5 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.

@garyc40
5 years ago

use full font stack

#13 in reply to: ↑ 12 @garyc40
5 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.

#14 @nacin
5 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.

#15 @nacin
5 years ago

Still having issues with long titles.

#16 follow-up: @dd32
5 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.

#17 in reply to: ↑ 16 @duck_
5 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.

#18 @dd32
5 years ago

I actually remember doing this ages ago.

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

#19 @ryan
5 years ago

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

#20 @ryan
5 years ago

wplink patches need refresh.

@garyc40
5 years ago

wplink patch refreshed

#21 @nacin
5 years ago

Latest wplink patch needs refresh again.

@garyc40
5 years ago

my bad, refreshed again

@garyc40
5 years ago

reverted change in wp-admin.css

@ocean90
5 years ago

Set font-size: 12px.

#22 @nacin
5 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.

#23 @dremeda
5 years ago

Seems good now Nacin.

#25 @nacin
5 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.

#26 @dremeda
5 years ago

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

#27 @nacin
5 years ago

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

#28 @nacin
5 years ago

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

#30 @ocean90
5 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

#31 @nacin
5 years ago

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

#32 @ocean90
5 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.

#33 @ocean90
5 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. :-)

#34 @nacin
5 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.