Opened 2 years ago
Closed 2 years ago
#15561 closed defect (bug) (fixed)
Press This is broken
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.1 |
| Component: | Press This | Version: | 3.1 |
| Severity: | normal | Keywords: | has-patch |
| Cc: | westi |
Description (last modified by westi)
- When using Press This, you are unable to add tags to your post.
- Link helper not working
...
Attachments (12)
Change History (46)
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?
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 :(
- 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:10
garyc40 — 2 years ago
- Keywords has-patch needs-testing added; needs-patch removed
comment:11
nacin — 2 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.
comment:12
follow-up:
↓ 13
nacin — 2 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.
comment:13
in reply to:
↑ 12
garyc40 — 2 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
nacin — 2 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.
comment:15
nacin — 2 years ago
Still having issues with long titles.
comment:16
follow-up:
↓ 17
dd32 — 2 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_ — 2 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
dd32 — 2 years ago
I actually remember doing this ages ago.
Me too, I had however, thought it was the normal admin category metabox :)
comment:19
ryan — 2 years ago
Per IRC discussion, all patches are good for commit. This ticket will be left open for further tweaks.
comment:20
ryan — 2 years ago
wplink patches need refresh.
comment:21
nacin — 2 years ago
Latest wplink patch needs refresh again.
comment:22
nacin — 2 years ago
comment:23
dremeda — 2 years ago
Seems good now Nacin.
comment:24
ocean90 — 2 years ago
comment:25
nacin — 2 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
dremeda — 2 years ago
I've checked latest Mac browsers (FF/Chrome/Safari/Opera).
comment:27
nacin — 2 years ago
Final checks for internal linking (IE6, RTL, long titles) need to be reported to #15739.
comment:28
nacin — 2 years ago
comment:29
nacin — 2 years ago
[17135] is designed to fix http://cl.ly/3a1k2y1I0a3I263j2a08.
comment:30
ocean90 — 2 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
nacin — 2 years ago
Any inkling as to why? I'll try to fire IE8 up on a machine.
comment:32
ocean90 — 2 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.
comment:33
ocean90 — 2 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
nacin — 2 years ago
- Resolution set to fixed
- Status changed from reopened to closed

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