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)

comment:1 @westi5 years ago

  • Description modified (diff)

@duck_5 years ago

comment: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

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

comment:4 @ryan5 years ago

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

comment: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 :(

comment:6 @scribu5 years ago

re maintenance: #14650

comment:7 @ryan5 years ago

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

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

comment:9 @ocean905 years ago

#15758 closed as a dup.

@garyc405 years ago

link dialog style fix

comment:10 @garyc405 years ago

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

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

@garyc405 years ago

move styles to wplink.css

comment:12 follow-up: @nacin5 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.

@garyc405 years ago

use full font stack

comment:13 in reply to: ↑ 12 @garyc405 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 @nacin5 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.

@nacin5 years ago

comment:15 @nacin5 years ago

Still having issues with long titles.

comment:16 follow-up: @dd325 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_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.

comment:18 @dd325 years ago

I actually remember doing this ages ago.

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

comment:19 @ryan5 years ago

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

comment:20 @ryan5 years ago

wplink patches need refresh.

@garyc405 years ago

wplink patch refreshed

comment:21 @nacin5 years ago

Latest wplink patch needs refresh again.

@garyc405 years ago

my bad, refreshed again

@garyc405 years ago

reverted change in wp-admin.css

@ocean905 years ago

Set font-size: 12px.

comment:22 @nacin5 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 @dremeda5 years ago

Seems good now Nacin.

comment:25 @nacin5 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 @dremeda5 years ago

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

comment:27 @nacin5 years ago

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

comment:28 @nacin5 years ago

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

comment:30 @ocean905 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 @nacin5 years ago

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

comment:32 @ocean905 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.

@ocean905 years ago

comment:33 @ocean905 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 @nacin5 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.